Skip to content
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

Fix extent computation in QgsVirtualLayerProvider #54976

Merged
merged 4 commits into from Oct 21, 2023

Conversation

strk
Copy link
Contributor

@strk strk commented Oct 18, 2023

Extent should be set to null if there are no rows or geometric field or computed min/max envelope ordinates are null.

Also makes the implementation more readable (hopefully).

Spin-off of GH-54646

Similar to GH-54917

@strk strk requested a review from nyalldawson October 18, 2023 13:15
@github-actions github-actions bot added this to the 3.34.0 milestone Oct 18, 2023
@strk strk added the Bug Either a bug report, or a bug fix. Let's hope for the latter! label Oct 18, 2023
Copy link
Collaborator

@nyalldawson nyalldawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test covering this? Looks like there's a coverage gap here....

@strk
Copy link
Contributor Author

strk commented Oct 19, 2023

Tests for this already exist in the python base class but only fail when the default constructor for Rectangle builds a Null rather than 0,0,0,0 rectangle - Maybe I should file a PR to do just that (build null) to trigger the many different issues and fix them all in the same PR ?

@strk
Copy link
Contributor Author

strk commented Oct 19, 2023

See the next commit pushed ( 75ce13e ) which would probably trigger failures in the existing testsuite if previous commit ( de747f5 ) was reverted.

I'd show via CI if force-pushing later would not invalidate the review ...

Extent should be set to null if there are no rows or geometric
field or computed min/max envelope ordinates are null.

Also makes the implementation more readable (hopefully).
@strk strk force-pushed the QgsVirtualLayerProvider-extent branch from cb007cf to 4d8243b Compare October 20, 2023 19:13
@strk strk merged commit f3e3e0d into qgis:master Oct 21, 2023
29 checks passed
@strk strk deleted the QgsVirtualLayerProvider-extent branch October 21, 2023 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants