Skip to content

Conversation

cvanelteren
Copy link
Contributor

This PR updates the logic for plotting on GeoAxes. Functions such as plot, scatter, pcolormesh, imshow, and others are now automatically projected to the appropriate coordinate reference system, making plotting more intuitive and seamless. Internally, we pass along the transform so that plotting on these axes works as expected, though this behavior may cause issues for some external packages. To address this, the inference order has been adjusted: if a transform is explicitly provided to the function, that transform is used; otherwise, the transform is inferred from the projection, defaulting to PlateCarree when unspecified.

Addresses #333

Copy link

codecov bot commented Aug 27, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cvanelteren
Copy link
Contributor Author

Need to check why pcolor functions are failing now but work when removing self.projection

@cvanelteren cvanelteren requested a review from Copilot August 27, 2025 06:56
Copilot

This comment was marked as outdated.

@cvanelteren cvanelteren marked this pull request as draft August 27, 2025 07:42
@cvanelteren cvanelteren requested a review from Copilot August 27, 2025 11:50
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements order projection inference for geo plotting in ultraplot, making plotting on GeoAxes more intuitive by automatically projecting functions to appropriate coordinate reference systems. The key improvement is a new precedence order: explicit transforms are prioritized, then axis projection (for imshow), and finally PlateCarree as default.

  • Updated transform inference logic to check for explicit user-provided transforms first
  • Added special handling for imshow function to use axis projection when available
  • Introduced fallback chain using PlateCarree as the final default transform

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
ultraplot/internals/inputs.py Implements new transform inference logic with precedence order and special imshow handling
ultraplot/tests/test_inputs.py Adds parametrized test to verify transform setting behavior with explicit and None values

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@cvanelteren
Copy link
Contributor Author

@beckermr do these changes make sense? The issue is (I think) t hat for imshow it will use PlateCarree as a reference which will put the imshow linearly from the data shapes i.e. (0, len(data), (0, len(data.T))

@cvanelteren cvanelteren requested a review from beckermr August 27, 2025 12:42
@cvanelteren cvanelteren marked this pull request as ready for review August 27, 2025 12:42
@beckermr
Copy link
Collaborator

We should add explicit tests of plotting with imshow to ensure things look ok.

@cvanelteren
Copy link
Contributor Author

Ok this does not work. The tests are fine but the imshow looks weird for non rectilinear projections. i.e.

PR (note that middle is now the same as left as it uses the projection see code below):
image

Before:

image

I don´t fully understand how PlateCarree works by wrapping inside the LLC projection

    data = rng.random((100, 100))
    fig, ax = uplt.subplots(ncols=3, proj="lcc", share=0)
    ax.format(land=True, labels=True)
    ax[:2].format(
        latlim=(-10, 10),
        lonlim=(-10, 10),
    )
    ax[0].imshow(data, transform=ax[0].projection)
    ax[1].imshow(data, transform=None)
    ax[2].imshow(data, transform=uplt.axes.geo.ccrs.PlateCarree())
    ax.format(title=["LLC", "No transform", "PlateCarree"])
    uplt.show(block=1)

Note that for cyl (rectilinear projections in general) the PR looks good:

image

@cvanelteren
Copy link
Contributor Author

I don't completely understand why before the PR the imshow is wrapper inside the image. From my understanding the imshow will place it at the coordinates which have the shape of the data; placing it in rectilinear confirms this. For the LCC projection I would not expect the entire space to be filled... kinda scratching my head on that one.

@beckermr
Copy link
Collaborator

Cool. In general, we need to be testing changes by making actual plots. We should be expanding the tests that compare actual figures.

@cvanelteren
Copy link
Contributor Author

The above is written in a test -- will push that now. Not sure what to do with the weirdness of the projection not working for the imshow ^ but it does work for #333

@beckermr
Copy link
Collaborator

So imshow has not way internally of knowing what range in lat and lon the data is supposed to cover, right? I guess this means it is assuming that the data ranges from 0 to len(array) and it is interpretting that in the context of the projection in some way?

What happens in cartopy or in mpl with the code above? That might help guide our understanding.

@cvanelteren
Copy link
Contributor Author

I will revert the inputs stuff and just add the two tests -- can´t hurt.

@cvanelteren
Copy link
Contributor Author

What happens in cartopy or in mpl with the code above? That might help guide our understanding.

From my understanding in #333 in matplotlib this does not happen. In our codebase inputs will set the transform which according to the src from contextily is not set for matplotlib explicitly which will mean that it will use matplotlib's default projection.

We pass on PlateCarree if transform is not given as an input, meaning all GeoAxes will use the same projection -- which confuses me. Note that when transform is given to matplotlib only code it will not change the plot. I believe they control the image size through extent directly

I am leaning towards just highlighting this to contextily as an edit in their code would make it compatible with us without creating headaches of finding where the alignment issue is coming from for the otherwise working codebase.

@beckermr
Copy link
Collaborator

Something is broken in the code above that is unrelated to the PR here. The range of longtiude is all in the west, but the image from imshow should be in the east and north. I think the axes ranges are not set so we can see what happens in the left most panel.

@cvanelteren
Copy link
Contributor Author

Something is broken in the code above that is unrelated to the PR here. The range of longtiude is all in the west, but the image from imshow should be in the east and north. I think the axes ranges are not set so we can see what happens in the left most panel.

Just to clarify I set the lim on the two left plots. When the projection is given, the imshow simply disappears for non-rectilinear projections.

@cvanelteren
Copy link
Contributor Author

How should we proceed you think?

@beckermr
Copy link
Collaborator

I want a better understanding of what is happening in each case before I comment on proceeding. I don't get why the imshow data shows up in some cases and not others, how transform is applied given a projection already exists, etc.

@cvanelteren
Copy link
Contributor Author

@beckermr I think we can merge this under a different title (just adding new tests). The reason for the mismatch is given in #333. We may want to add some description in the docs on this to this PR.

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.

3 participants