-
Notifications
You must be signed in to change notification settings - Fork 142
Fix annotation projections #130
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
Conversation
44d525a to
4a6effa
Compare
| connection_file = os.path.basename(connection_file_path) | ||
| return connection_file.split('-', 1)[1].split('.')[0] | ||
|
|
||
| def transform_coordinates(source_srs, target_srs, x, y): |
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.
@dorukozturk we probably want to add a array based as well as that would be more efficient, passing one coordinate at a time would be slow if we want to have it as general util.
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.
also does it makes sense to have it (x, y, target_srs) and source_srs arg if not provided can be assumed to wgs84?
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.
At first that is what I did. Then I realized it would be a better utility function with the ability to use it the other way around (converting to wgs84). It takes an array of coordinates now.
|
|
||
| def index(self, *args, **kwargs): | ||
| target_srs = self.dataset.crs | ||
| # Annotations are always in WGS84 |
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.
Annotations are in wgs84 for now but that may change in the future (distant future)
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 agree. That is why I added a comment. It could change in the future. As long as we know the annotation projection we can use the utility function for transforming coordinates
3f012c0 to
dc7ded2
Compare
kotfic
left a comment
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.
LGTM -
FYI it seems like the docker build is crashing, I will look into it and handle in a different PR
When getting a subset using an annotation, if the source tiff is not in WGS we were getting an empty array. This adds a utility function to transform coordinates of the annotation to the source image's coordinates. Fixes #129