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 commit 61f1815 #14

Merged
merged 4 commits into from
Dec 6, 2023
Merged

fix commit 61f1815 #14

merged 4 commits into from
Dec 6, 2023

Conversation

metzm
Copy link
Contributor

@metzm metzm commented Dec 1, 2023

In commit 61f1815 for PR #11, the import flags in import_single_alkis_source() have been deactivated, but two instances have not been commented out. As a result, flags becomes automatically initialized to invalid d,r when reaching L384 and L392. This PR fixes this.

@metzm metzm added the bug Something isn't working label Dec 1, 2023
@anikaweinmann
Copy link
Member

@metzm since the tests are not run automatically, the question is they still work with the changes?

@metzm
Copy link
Contributor Author

metzm commented Dec 5, 2023

The tests already failed for PR #11, I had to re-enable the hack for Hessen. Regarding Hessen, a better solution would be to assign the correct CRS to the input data as done for Thüringen, see new code comment.

Regarding Thüringen, any reason why a CRS is assigned to the input data only if an aoi_map is given?

@anikaweinmann
Copy link
Member

Regarding Thüringen, any reason why a CRS is assigned to the input data only if an aoi_map is given?

@metzm Can we simply change https://github.com/mundialis/v.alkis.buildings.import/blob/fix_import_flags/v.alkis.buildings.import.py#L329C35-L329C40 and us the EPSG code of the current location?

@metzm
Copy link
Contributor Author

metzm commented Dec 5, 2023

Regarding Thüringen, any reason why a CRS is assigned to the input data only if an aoi_map is given?

@metzm Can we simply change https://github.com/mundialis/v.alkis.buildings.import/blob/fix_import_flags/v.alkis.buildings.import.py#L329C35-L329C40 and use the EPSG code of the current location?

This could work since in the Thüringen case ogr2ogr -t_srs EPSG:25832 attempts to reproject the vector which currently has no effect because the vector is already in EPSG:25832.

But then, why not just replacing v.in.ogr with v.import?

@metzm
Copy link
Contributor Author

metzm commented Dec 6, 2023

I have changed the special treatment of Hessen and Thüringen to the best of my knowledge. All tests are passed.

Copy link
Member

@anikaweinmann anikaweinmann left a comment

Choose a reason for hiding this comment

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

Thank you :)

@metzm metzm merged commit 69cd54b into main Dec 6, 2023
2 checks passed
@metzm metzm deleted the fix_import_flags branch December 6, 2023 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants