-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
[Bug]: Markers with string-type integer values are converted to integers in cycler and matplotlibrc #29646
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
The source of this is likely that this code is also used to validate reading |
Yes, the validators accept two kinds of inputs: (1) a python object suitable for the context, (2) a string that can be evaluated to such a python object. The latter is needed because Anyway, a solution should make the following modification to the tests pass.
diff --git a/lib/matplotlib/tests/test_cycles.py b/lib/matplotlib/tests/test_cycles.py
index 4fa2616194..587d4f0509 100644
--- a/lib/matplotlib/tests/test_cycles.py
+++ b/lib/matplotlib/tests/test_cycles.py
@@ -29,7 +29,10 @@ def test_marker_cycle():
def test_valid_marker_cycles():
fig, ax = plt.subplots()
- ax.set_prop_cycle(cycler(marker=[1, "+", ".", 4]))
+ ax.set_prop_cycle(mpl.cycler(marker=["1", "+", ".", 4]))
+ for _ in range(4):
+ ax.plot(range(10), range(10))
+ assert [l.get_marker() for l in ax.lines] == ["1", "+", ".", 4]
def test_marker_cycle_kwargs_arrays_iterators():
diff --git a/lib/matplotlib/tests/test_rcparams.py b/lib/matplotlib/tests/test_rcparams.py
index 096c5cef6c..44be6a9929 100644
--- a/lib/matplotlib/tests/test_rcparams.py
+++ b/lib/matplotlib/tests/test_rcparams.py
@@ -268,6 +268,8 @@ def generate_validator_testcases(valid):
("cycler('c', 'rgb') * cycler('linestyle', ['-', '--'])",
(cycler('color', 'rgb') *
cycler('linestyle', ['-', '--']))),
+ ('cycler(marker=["1", 2])',
+ cycler(marker=["1", 2])),
(cycler('ls', ['-', '--']),
cycler('linestyle', ['-', '--'])),
(cycler(mew=[2, 5]), That said, a workaround for you would be to use the unvalidated |
Prior to #27613 (mpl 3.8.3/3.9.0), it was not possible to specify integer markers (TICKLEFT=1, etc.) in the matplotlibrc file, which uses strings only (or in a cycler, #27596). That PR "fixed" the issue by instead interpreting the string "1" as an integer, but the consequence is that it has now become impossible, instead, to specify string markers whose string shorthand is an integer (this issue). Given that it is anyways confusing to have two different markers called Perhaps we could also deprecate the int-as-str markers ( |
Introducing full names for the markers is a good idea. That makes marker definition unambiguous in every situation. We can separately decide whether we want to deprecate the current forms. I would not revert anything. AFAICS the before/after of #27613 are just the two choices that a validator can make - convert "2" to int or not. Neither seems better than the other. I therefore would leave it like this. Though I suspect it should be possible to fix this by jumping some extra hoops
And also, we should move away from plain-text matplotlibrc. But that's a separate topic. |
The point is that I believe that one of the choices (don't convert) is better than the other, for more than one reason:
|
I appreciate all of the input, especially the patch for tests from @timhoffm.1 In fact, the reason I found out about this bug was because my mplstyle file was being processed incorrectly. I had traced the issue to validation in Note that this bug actually affects the following four additional rcParams entries, where
These rcParams entries all behave differently pre- and post-#27613, as they all use I agree with @timhoffm, that it would be better to "[...] separate conversion from validation". However, that may require some re-architecting of how matplotlibrc is loaded and parsed. For example, double-quoting would be a nice way of indicating a string but double-quotes are removed in A few notes:
It would be great if the following matplotlibrc/mplstyle file was possible.
Footnotes |
Bug summary
When creating a cycler with a
"marker"
property value contains strings that could be converted to integers, they are always converted to integers, even if the string value is valid and the intended value. This is most obvious with the list["1", "2"]
; which is distinctly different from[1, 2]
. However, the latter is always used. Edit (20.02.2025): This is also the case for"lines.markers"
,"boxplot.flierprops.marker"
,"boxplot.meanprops.marker"
, and"scatter.marker"
.Relevant functions are
_validate_markers
(here) and_listify_validator
(here). It seems like this issue was introduced by #27613, which solved a similar issue reported in #27596.Code for reproduction
Edited 20.02.2025
Actual outcome
which indicates that the string entries
"1"
and"2"
are converted into integers. As the string"o"
cannot be converted into an integer withint("o")
, it is used explicitly.Expected outcome
This output would indicate that strings containing integers are parsed correctly.
Additional information
This bug was not present prior to #27613, but it did solve a similar bug.
Looking at the code which causes this bug,
_validate_markers
(here) and_listify_validator
(here), it's not clear to me why the validation is done this way. I.e., to validate types, values are explicitly converted to see if aValueError
is raised, instead of testing the type explicitly. Is this the intended method for validation?I have a draft solution, but I would like clarification on the intended validation mechanisms before I submit the code for review.Edit (20.02.2025): Since finding that other RcParams entries are affected by this bug, my solution is no longer applicable.Operating system
Fedora 41
Matplotlib Version
3.10.0
Matplotlib Backend
No response
Python version
Python 3.12.9
Jupyter version
No response
Installation
pip
The text was updated successfully, but these errors were encountered: