-
Notifications
You must be signed in to change notification settings - Fork 2
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
CRS bug fix with temporary subset_geom clipping object, original get_target_crs function #125
base: main
Are you sure you want to change the base?
Changes from all commits
c2f90a0
77ae382
34dde09
0f468d9
034fc19
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I remember talking about this change, but I don't remember enough to feel confident this is the entire solution we wanted. Plus, we talked more recently about target CRS versus user-provided CRS, so does that affect this change here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The actual CRS bug was fixed previously in this commit: 3dc613b. The CRS related-change here improves upon that fix by creating a temporary geometry instead of modifying the request object's geometry to prevent downstream issues. Switching to using separate target_crs and user_geom_crs parameters will be a rather time consuming overhaul of the code, so I propose we fix this small issue now, and fix the larger issue of separating target_crs into two different parameters once we have the capacity to take that on. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -162,5 +162,3 @@ def get_target_crs(input_crs_str, resolution, user_geom): | |
|
||
return target_crs | ||
|
||
|
||
|
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.
The change here overlaps with the #126 PR to which I committed a further edit.