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 getproperty(Row, :geometry) #83

Merged
merged 2 commits into from
Jan 26, 2023
Merged

Fix getproperty(Row, :geometry) #83

merged 2 commits into from
Jan 26, 2023

Conversation

felixcremer
Copy link
Member

This change allows to access the geometry of a Shapefile.Row via Row.geometry.
This PR fixes #82.
This change is also needed so that Tables.columns of a collection of Shapefile.Row works (which you can end up with when filtering a Shapefile by some column values).

Why is Base.propertynames(Row) overloaded so that it doesn't also show the :geometry column?
I thought about also changing that, but it is actively tested for, therefore I was hesitant to change it.

This change allows to access the geometry of a Shapefile.Row via Row.geometry.
@rafaqz
Copy link
Member

rafaqz commented Jan 25, 2023

I think we have to change propertynames if we change getproperty.

From the propertynames docs:

Get a tuple or a vector of the properties (x.property) of an object x. This is typically the same as fieldnames(typeof(x)), but types that overload getproperty should generally overload propertynames as well to get the properties of an instance of the type.

So go ahead and change the test ;)

@felixcremer
Copy link
Member Author

I went ahead and changed the test. ;-)
And the function as well.

@rafaqz
Copy link
Member

rafaqz commented Jan 26, 2023

One question I have is how this interacts with round trip read/write - I guess DBFTables.jl just ignores the :geometry column in the Table?

@joshday are there any issues with merging this in relation to your work on write ?

@joshday
Copy link
Contributor

joshday commented Jan 26, 2023

Shouldn't be any issues on writing. Shapefile.write drops any column with elements that satisfy GeoInterface.isgeometry for the dbf file.

@rafaqz rafaqz merged commit c7b41f3 into main Jan 26, 2023
@rafaqz rafaqz deleted the fc/rowproperty branch January 26, 2023 15:16
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.

Accessing a Shapefile.Row geometry via row.geometry fails
3 participants