-
Notifications
You must be signed in to change notification settings - Fork 7
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
Deprecation fix mpl 3.10 and beyond #69
base: main
Are you sure you want to change the base?
Conversation
Making this a draft as we have not updated to 3.10 yet. |
Note I had to rename the warning UltraplotWarning to UltraPlotWarning due to some weird pytest-xdist issue. |
I had some spare time so pushed this along. There is one warning I could use your input for. There is a warning for the boxplot that keyword The warning implies that properties have to be fed through the The major problem is that it is not trivial to determine from a singular entry point how to cycle general objects. Line objects have different properties from boxplots, and contours. As such, setting all the properties through a general cycler won't be possible unless we write the code for this. There are some properties that can be leveraged for this in In short, we either For P.S. This warning can be found by running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments.
@@ -3892,6 +3892,7 @@ def _apply_boxplot( | |||
x, y, autoy=False, autoguide=False, vert=vert, **kw | |||
) | |||
kw = self._parse_cycle(x.size, **kw) # possibly apply cycle | |||
print(kw) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be removed before merging.
Handle deprication of basemap and cartopy package. | ||
Handle potential deprication of basemap and cartopy package. | ||
""" | ||
if basemap is not None: | ||
backend = ("cartopy", "basemap")[bool(basemap)] | ||
warnings._warn_ultraplot( | ||
f"The 'basemap' keyword was deprecated in version 0.10.0 and will be " | ||
f"removed in a future release. Please use backend={backend!r} instead." | ||
) | ||
# Basemap is currently being developed again so are removing the deprecation warning |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not right. This deprecation warning was about how to specify the backend (via a string) and does not depend on whether or not basemap is being developed. We need to keep this code and force folks to write backend='basemap'
if they want basemap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The backend is not getting removed imo. At the time Luke developed this basemap was EOL.
@@ -150,14 +150,15 @@ def test_boxplot_vectors(): | |||
datas = np.array(datas, dtype=object) | |||
assert len(datas) == len(coords) | |||
fig, ax = uplt.subplot(refwidth=3) | |||
cycle = uplt.Cycle("538") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason the cycle is made directly here?
Hi @beckermr thanks for the comment but they don't address the things I outlined. |
Yes I know, sorry. I don't fully understand why things were deprecated either. I think not deprecating it fow now is fine. |
There are a lot of deprecation warnings from MPL and or Ultraplot existing in the current build. The majority of these fixes can be addressed by modifying the tests. Some of the deprecation warnings ensures keywords of usage to be compatible with mpl 3.10 which will result in errors in later versions.
There are a few errors I have not addressed that warrants some discussion.
The first one
The second one
For both of these, I don't agree with the deprecation warning. The first issue indicates that we should move towards a cycler object. However, adjusting the subplot to match the target baseline cannot be readily achieved through the cycler object. It requires setting the patches to a different alpha and leaving the lines with alpha = 1. This would (I think) require an extension of what the cycler object can handle which is to set all potential properties of all the artists. I don't think it can currently do this.
The second error implies the removal of multi-axis formatting. Personally, I would be against removing this feature as it is one of the most powerful features of UltraPlot.