-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Test and fix QgsRectangle::isNull #45607
Conversation
: mXmin( std::numeric_limits<double>::max() ) | ||
, mYmin( std::numeric_limits<double>::max() ) | ||
, mXmax( -std::numeric_limits<double>::max() ) | ||
, mYmax( -std::numeric_limits<double>::max() ) |
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'm wondering if it's cleaner to just set these all as nan, and adjust the isnull test accordingly. What's your thoughts?
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 don't think it matters, as long as callers all check for isNull
I think this is heading in the right direction, nice work @strk ! |
There are 15 failing tests. One of them (testClipRasterByExtent) reports:
I would think The test is in python/plugins/processing/tests/GdalAlgorithmsRasterTest.py |
That gdal_translate test is very suspiciou, because it expects gdal_translate being invoked with a |
Update expected QgsRectangle::toString output on Null rectangle
Phew, that must be from years ago, do you have a link to the code or the PR that added it? |
Nope, and the testsuite had a reorganization at some point (2019) which makes it harder to tell. |
The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check
|
While we hate to see this happen, this PR has been automatically closed because it has not had any activity in the last 21 days. If this pull request should be reconsidered, please follow the guidelines in the previous comment and reopen this pull request. Or, if you have any further questions, just ask! We love to help, and if there's anything the QGIS project can do to help push this PR forward please let us know how we can assist. |
Reopening as bug #45563 which this PR attempts to address, is still existing |
Reopening this PR didn't work, so I'm creating a new one replacing this |
Superceeded by #47404 |
Adds tests for QgsRectangle::isNull, improve toString to show the difference between Null and Empty, eventually fix the 0,0,0, rectangle case to still be Empty and NOT Null (to match documentation)