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

API: GH #2013 make regrid arguments, docstring consistent #2014

Merged
merged 5 commits into from
Mar 26, 2022

Conversation

ellequelle
Copy link
Contributor

@ellequelle ellequelle commented Mar 10, 2022

Rationale

Addresses #2013

Implications

This is a change to the API for img_transform.regrid

I changed the name of the 4th argument from source_cs to source_proj to be consistent with other argument names. It's probably uncommon for source projection object to be passed as a keyword argument since it's the fourth of six positional arguments.

Old function signature:

def regrid(array, source_x_coords, source_y_coords, source_cs, target_proj,
target_x_points, target_y_points, mask_extrapolated=False):

New function signature:

def regrid(array, source_x_coords, source_y_coords, source_proj, target_proj,
           target_x_points, target_y_points, mask_extrapolated=False)

lib/cartopy/img_transform.py Outdated Show resolved Hide resolved
lib/cartopy/img_transform.py Outdated Show resolved Hide resolved
lib/cartopy/img_transform.py Outdated Show resolved Hide resolved
@dopplershift dopplershift added the API Change Denotes issues or PRs that change the interface label Mar 10, 2022
@dopplershift dopplershift added this to the 0.21 milestone Mar 10, 2022
@ellequelle ellequelle changed the title GH #2013 make regrid arguments, docstring consistent API: GH #2013 make regrid arguments, docstring consistent Mar 10, 2022
@ellequelle ellequelle marked this pull request as ready for review March 10, 2022 20:49
Copy link
Contributor

@greglucas greglucas left a comment

Choose a reason for hiding this comment

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

👍 I think this has little chance of impacting people downstream and isn't worth doing a lot of deprecation dance around.

@dopplershift
Copy link
Contributor

I agree, my only concern is the test failures, which I've not had a chance to dig into to see if they're related.

@ellequelle
Copy link
Contributor Author

I've been confused by this one, but I get an identical error (cartopy.tests.test_shapereader.TestLakes) when testing the head branch on my computer, so it doesn't seem to be due to any of these changes.

@greglucas
Copy link
Contributor

Yes, that looks like the same error as here: #2012
which is caused by pyshp, and our minimum dependencies is the only location where we are using pyshp.

@greglucas greglucas merged commit f1d72d1 into SciTools:main Mar 26, 2022
@greglucas
Copy link
Contributor

Thanks, @ellequelle!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Change Denotes issues or PRs that change the interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants