Add explicit converter setting to Axis #28970
Conversation
|
Does this also need a |
| axis._set_converter(axis_converter[name]) | ||
| axis.set_units(axis_units[name]) |
There was a problem hiding this comment.
The choice to use the private setter was a defensive one as the public
setter prevents being called multiple times (though does short circuit
if an identical input is provided, which I think is actually true here,
therefore the public one is probably functional (and a no-op).)
It is not clear to me on analysis how the unit information is or was
lost. A quick test commenting out these two lines which reset
converter/units displayed no obvious detrimental effect to removing
them, suggesting that even if once they were necessary, they may no
longer be.
These lines were last touched in #24141, though that really only generalized
the code into a loop rather than copy/pasted x and y behavior.
The original inclusion of resetting was in #4909, which indicated that
the dialog reset unit info. AFAICT, that is no longer true, though I
have not rigorously proved that.
The replacement is the get/set_converter method. This includes changes to use the getter and the private setter in the qt figure options dialog menu. The choice to use the private setter was a defensive one as the public setter prevents being called multiple times (though does short circuit if an identical input is provided, which I think is actually true here, therefore the public one is probably functional (and a no-op).) It is not clear to me on analysis how the unit information is or was lost. A quick test commenting out the two lines which reset converter/units displayed no obvious detrimental effect to removing those, suggesting that even if once they were necessary, they may no longer be. These lines were last touched in matplotlib#24141, though that really only generalized the code into a loop rather than copy/pasted x and y behavior. The original inclusion of resetting was in matplotlib#4909, which indicated that the dialog reset unit info. AFAICT, that is no longer true, though I have not rigorously proved that.
862d275 to
493fbc6
Compare
|
Going to merge over the failure and created an issue to look into the Qt editor issues. |
Closes #19229
PR summary
Adds an API to explicitly set the converter for an Axis.
Once explicitly set, the implicit machinery will not override the converter.
Previously (and still, if not explicitly set), the converter would be reset each time new data was added, possibly setting to something incompatible with previously provided data.
You cannot reset the converter once explicitly provided (except by clearing the axis entirely).
A warning is provided if implicitly set and being set to something different.
PR checklist