Note that I know this looks weird - but this in an application library that where the user may or may not set units. So we always pass that info though to the kwarg and it may be None in some cases.
The issue is in axes/_base.py:2130 in the _process_unit_info() method. If the xunits or yunits kwarg is set, it's used even if it's None. The question here is what is None in this context. Before 2.2.2, None meant "use the default units". But now that's not possible because it depends on what was plotted before that call whether that will work or not.
My suggestion is that None should mean "I'm not setting this" and behave the way it did before. I think that adding these lines to process_unit_info would fix this (repeat for yunits).
I think the problem is that string categorical is stashing a units mapping object unit. The information you're passing is a string, and so maybe it should be stashed as an attribute on the units object?
Sorry - I don't understand what you mean. What is "the information you're passing"? I think it's important to answer the question: what does passing xunits=None mean? Then we can talk about solutions and code. This is probably related to the changes that were made for locking units to an axis once they are set but I think None wasn't handled in that case.
I don't think the units-locking PR ever got merged... (#9776), but a bunch of stuff was done to categoricals, which this issue would seem to touch because you are passing string arguments in...
@TD22057 sorry, I meant that the yaxis data is ["V1", "V1"], which means it hits the StrCategorical code path. In the StrCategorical code path, the axis.units attribute gets set to a UnitData object:
Probably, but I think it raises an issue of what happens when xunit or yunit isn't none but is a string. The JPL string implementation uses it to store sorting (which I think is a smart idea), but currently any use of it will clobber the object on strings.
I've added a nicer error message in #17824. It seems that unless we want to enforce None as a "do-nothing" unit in Matplotlib (which would be a big API change if external libraries have converters that take None as a valid unit), this isn't fixable.
@TD22057 Sorry this fell by the wayside for so long.
@dstansby that is not quite what is being proposed. Axis.units is inited to None if we explicitly call axes.set_units(None) it would still be cleared, what @TD22057 is asking for is if you pass xuints=None to ax.plot it gets treated as "not really passed" / "do the the default thing" (which is this case is "do no change the units") which seems like the correct API (you can change but not clear) the units from ax.plot. I suspect that this has "worked" in most cases because most other converters are not storing useful / critical information in the units attribute.
This first stopped working (but with a slightly different exception that may never have been released) in 34b8eb4 which changed from storing the string <-> float mapping in a second attribute to re-using the 'units` attribute (which seems to be a defend-able position, but I think we may have miss-understood how it was generally being used).
Activity
story645 commented on Apr 23, 2018
I think the problem is that string categorical is stashing a units mapping object unit. The information you're passing is a string, and so maybe it should be stashed as an attribute on the units object?
TD22057 commented on Apr 24, 2018
Sorry - I don't understand what you mean. What is "the information you're passing"? I think it's important to answer the question: what does passing xunits=None mean? Then we can talk about solutions and code. This is probably related to the changes that were made for locking units to an axis once they are set but I think None wasn't handled in that case.
jklymak commented on Apr 24, 2018
I don't think the units-locking PR ever got merged... (#9776), but a bunch of stuff was done to categoricals, which this issue would seem to touch because you are passing string arguments in...
story645 commented on Apr 24, 2018
@TD22057 sorry, I meant that the yaxis data is ["V1", "V1"], which means it hits the StrCategorical code path. In the StrCategorical code path, the axis.units attribute gets set to a UnitData object:
So xunits or yunits = "None" now overwrites that object.
tacaswell commented on Apr 24, 2018
I think @TD22057 's suggestion is correct, we should ignore None when passed as xunit or yunit.
story645 commented on Apr 24, 2018
Probably, but I think it raises an issue of what happens when xunit or yunit isn't none but is a string. The JPL string implementation uses it to store sorting (which I think is a smart idea), but currently any use of it will clobber the object on strings.
dstansby commented on Jul 2, 2020
I've added a nicer error message in #17824. It seems that unless we want to enforce None as a "do-nothing" unit in Matplotlib (which would be a big API change if external libraries have converters that take None as a valid unit), this isn't fixable.
tacaswell commented on Jul 3, 2020
@TD22057 Sorry this fell by the wayside for so long.
@dstansby that is not quite what is being proposed. Axis.units is inited to None if we explicitly call axes.set_units(None) it would still be cleared, what @TD22057 is asking for is if you pass xuints=None to ax.plot it gets treated as "not really passed" / "do the the default thing" (which is this case is "do no change the units") which seems like the correct API (you can change but not clear) the units from ax.plot. I suspect that this has "worked" in most cases because most other converters are not storing useful / critical information in the units attribute.
This first stopped working (but with a slightly different exception that may never have been released) in 34b8eb4 which changed from storing the string <-> float mapping in a second attribute to re-using the 'units` attribute (which seems to be a defend-able position, but I think we may have miss-understood how it was generally being used).
API: treat xunits=None and yunits=None as "default"
API: treat xunits=None and yunits=None as "default"
API: treat xunits=None and yunits=None as "default"
API: treat xunits=None and yunits=None as "default"
API: treat xunits=None and yunits=None as "default"