-
Notifications
You must be signed in to change notification settings - Fork 368
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
MNT: Switch to (lon, lat) for all argument and return ordering #2453
Conversation
There are more usages of (lon, lat) currently and only a few private versions of (lat, lon) so it is easy enough to switch everything over to (lon, lat) for consistency.
@@ -36,7 +36,7 @@ def test_osni(self, approx): | |||
ll = ccrs.Geodetic() | |||
|
|||
# results obtained by nearby.org.uk. | |||
lat, lon = np.array([54.5622169298669, -5.54159863617957], | |||
lon, lat = np.array([-5.54159863617957, 54.5622169298669], |
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.
Why is this even put through np.array
in the first place if it's split immediately?
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 was wondering the same thing, but really didn't want to go refactoring more than this PR title suggests.
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.
Curious what the failure cause is?
Looks like an issue with a corrupted shapefile download or something that Fiona is causing errors from which leads to a later error.
I reran the job as this seems like something intermittent and unrelated. |
There are more usages of (lon, lat) currently and only a few private versions of (lat, lon) so it is easy enough to switch everything over to (lon, lat) for consistency. This matches what we are expecting from transform_point(x, y) as well.