-
Notifications
You must be signed in to change notification settings - Fork 43
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
cleaning and error in the gdal file #916
Conversation
I think this was still working because most of the pixels have a square shape
if bottom is None: | ||
bottom = bounds['ymin'] if width is None else top + width | ||
bottom = bounds['ymin'] if height is None else top - height |
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.
- and + where inverted as well. We agree that height is positive ?
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.
width
should be height
in the bottom =
line, but bottom > top depends on the coordinate system. I'll have to investigate further.
left if right is None else right, | ||
top if bottom is None else bottom, | ||
units) | ||
pleft, ptop = self.toNativePixelCoordinates(left or right, top or bottom, units) |
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.
reduce complexity with nice use of the "or" keyword
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.
None != 0, so this doesn't actually do the same thing. Specifically, this is if right is not specified, use left as a fall-back. You might want a 0 coordinate depending on the crs (e.g., it could be the prime meridian of your reference system).
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.
sorry didn't thought about actual zero value (Flake8 would have complained anyway)
@12rambau Thanks for working on this. |
what's the use of this lock ? It's never called in this file |
This has been superceded by #1115. |
As I'm working on the creation of a rasterio provider I need to fully understand the gdal file. I have a lot of questions so I oppened this draft to notify you of my findings.
Please comment if something I post here is not making sense (I'll use the comment functionality of the PR to link code).
When I'll be done I'll remove the draft mode and I assume you'll squash my commits. What do you think ?