Skip to content
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

oceanspy using background_patch (and other) deprecated cartopy properties #310

Closed
Mikejmnez opened this issue Jan 18, 2023 · 5 comments · Fixed by #311
Closed

oceanspy using background_patch (and other) deprecated cartopy properties #310

Mikejmnez opened this issue Jan 18, 2023 · 5 comments · Fixed by #311
Assignees

Comments

@Mikejmnez
Copy link
Collaborator

  • OceanSpy version:
    0.2 and 0.3.2

Description

I ran the notebooks within the docs folders (as well as those in binder) with the new environment. All run fine except Particles.ipynb. The problem is that the newer versions of cartopy (version > 0.20.2) have deprecated properties that oceanspy still currently uses. Particularly:

background_patch
xlabels_top
ylabels_right

Running Particles.ipynb in Sciserver with current compute image Oceanography (Integrated Viewer), you can see get a deprecated warning. See the snapshot below:

Particles_notebook_carto_0p20p2_matpl_3p5p2

Starting with Cartopy version 0.21 (and matplotlib 3.5.2), Particles.ipynb already fails to complete.

Solution

As you can see in the snapshot above, we need to replace, the above properties by

GeoAxes.patch
.top_labels
.right_labels

I will work on this in the next couple of days. Then make a new (patch) release with these changes so that we can properly update the image in Sciserver.

@Mikejmnez Mikejmnez self-assigned this Jan 18, 2023
@Mikejmnez
Copy link
Collaborator Author

There is a small chance that only the documentation notebook needs to change. I'm looking into this..

@Mikejmnez
Copy link
Collaborator Author

Ok. just making the change in the notebook (replace background_patch with .patch), gets you the following plot:

result

which does not have ticks nor labels on the axes. You can compare it against the snapshot above (first comment), which uses an older version of cartopy (it does have the ticks and labels).

There is no import error, because oceanspy only tries to use the deprecated axes properties from cartopy. The relevant code is on oceanspy.plot, lines 758-762, and it reads like:

                try:
                    gl = ax.gridlines(crs=transform, draw_labels=True)
                    gl.xlabels_top = False
                    gl.ylabels_right = False

This is the only place these deprecated properties are used.

Solution

I will make the change to the notebook and the above snippet of code, and get a PR going in a minute.

@Mikejmnez
Copy link
Collaborator Author

Mikejmnez commented Jan 19, 2023

Turns out that the issue of ticks and labels not appearing, is because by default

add_labels=False

when plotting with od.plot.horizontal_section()

I guess it used to be that add_labels=True by default, and so tick lines and labels appeared without the need to set this argument True ? (see here https://oceanspy.readthedocs.io/en/latest/Particles.html). Now, you have to specify them as an argument.

So now, in the example above,. we have to set it manually true as follows

    fig = plt.figure(figsize=(10, 5))
    ax = od_eul.plot.horizontal_section(varName="masked_Depth", cmap="bone_r", add_labels=True)
    land_col = (253 / 255, 180 / 255, 108 / 255)
    ax.patch.set_facecolor(land_col)
    ax.set_extent([-40, -19, 63, 68])

which produces the following plot

plot

It is still true about the need to address the deprecation errors (specially in the notebooks!). But these do not need to happen before updating the Oceanography image in SciServer.

I will continue to work on the PR. But there is no need to make a new release inmediately after the PR.

@ThomasHaine
Copy link
Collaborator

Thanks @Mikejmnez ! All looks good.

@Mikejmnez
Copy link
Collaborator Author

I wasn't finished... but it is OK I guess. This turned out not to be critical.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants