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

changed check for validity from isGeosValid() to validateGeometry() #2935

Merged
merged 4 commits into from Mar 23, 2016
Merged

changed check for validity from isGeosValid() to validateGeometry() #2935

merged 4 commits into from Mar 23, 2016

Conversation

ghost
Copy link

@ghost ghost commented Mar 21, 2016

changed check for validity from isGeosValid() to validateGeometry() in dissolve.py.

Second try, as the first one failed due to wrong usage of git.
The background is that complicated polygons (e.g. created from vectorize, like innerringtouchesouterring.gml ) may contain "invalid" geometries according to GEOS , mainly because of self-intersections of the outer ring. QgsGeometry::checkValidity is more robust and only sorts out the truly defective geometries. Attached are tests with invalid and valid geometries. I hope this time I'm doing better in committing :)
The gml output files having defective (NULL) CRS is another bug somewhere else, #14544.

OUTPUT:
type: vector
name: expected/PolygonDissolveTest_output.gml

Copy link
Member

Choose a reason for hiding this comment

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

There seems to be an indentation problem here:

Compare this test:

  - name: dissolve
  algorithm: qgis:dissolve
  #input is valid as validateGeometry(), but invalid in isGeosValid()
  params:

to the previous (working one)

  # case 3: merge with longest common boundary
  - algorithm: qgis:eliminatesliverpolygons
    name: Eliminate sliver polygons largest area

DISSOLVE_ALL: 'True'
FIELD: None
INPUT:
name: custom/nullGeometryDissolve.gml
Copy link
Member

Choose a reason for hiding this comment

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

the file is currently not located in the subfolder custom

@m-kuhn
Copy link
Member

m-kuhn commented Mar 22, 2016

  • The nullgeometrydissolve exptected output file seems to be missing
  • Could you move the dataset into custom and revert the path?
  • There are some other indentation "problems" which get rejected by autopep8: https://travis-ci.org/qgis/QGIS/jobs/117682954#L1063-L1112 if you plan to provide more fixes, please read the bottom of the selected lines to setup your local git to auto-fix such things in the future. If not, I can do this quickly for you when it's being merged.

@m-kuhn m-kuhn merged commit bec849c into qgis:master Mar 23, 2016
@m-kuhn
Copy link
Member

m-kuhn commented Mar 23, 2016

Thank you very much, such pull requests are valuable!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant