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

Reinstate missing renderer argument in GeoAxes._preprocess_draw #2465

Closed
wants to merge 2 commits into from

Conversation

jfrost-mo
Copy link

@jfrost-mo jfrost-mo commented Oct 29, 2024

Rationale

Fixes #2464.

I've reimplemented what I believe was the intent of 454567a from PR #2424, in such a way that it no longer changes the method signature in a problematic way.

Implications

This should now be fully compatible with existing code.

I have manually tested this change however I haven't added any automatic tests. If one is needed then I'm happy to create one.

@jfrost-mo jfrost-mo changed the title Missing renderer argument Reinstate missing renderer argument in _preprocess_draw Oct 29, 2024
@jfrost-mo jfrost-mo changed the title Reinstate missing renderer argument in _preprocess_draw Reinstate missing renderer argument in GeoAxes._preprocess_draw Oct 29, 2024
@jfrost-mo jfrost-mo force-pushed the missing_renderer_argument branch 2 times, most recently from 667fcc2 to 6a62af9 Compare October 29, 2024 13:39
Commit 454567a changes the method signature
of GeoAxes._preprocess_draw. This causes TypeErrors in user code that expects
draw to take a renderer argument, notably iris code with bbox_inches="tight".

This commit reinstates the argument in the _draw_preprocess function to make
the method signature compatible. The value of the argument is never used in
the function.
@rcomer
Copy link
Member

rcomer commented Oct 29, 2024

Thanks for your willingness to contribute @jfrost-mo!

What Iris version did you test this with? My understanding of the problem is that it happens when get_tightbbox or draw calls Iris’s patched _draw_preprocess without the parameter (which the patched method expects), so I would expect any fix to be in the callers.

However, since the problem is already fixed in the Iris release candidate, I’m not sure it makes sense to also fix it here.

@rcomer
Copy link
Member

rcomer commented Oct 30, 2024

Right, so when you tested this Cartopy branch, was that definitely with Iris 3.10? I think there was some sort of glitch that meant conda-forge users were getting Iris 3.11rc without deliberately selecting it.
SciTools/iris#5571 (reply in thread)

@jfrost-mo
Copy link
Author

I was using the given conda lockfiles, including hashes, so unless you somehow managed to get around that it should have been 3.10.0.

@jfrost-mo
Copy link
Author

This has been resolved in iris 3.11.0rc0.

@jfrost-mo jfrost-mo closed this Oct 30, 2024
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 this pull request may close these issues.

Cartopy 0.24 throws a TypeError when saving with bbox_inches="tight"
2 participants