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 schema mapping if driver includes default fields #918

Merged
merged 2 commits into from
Aug 2, 2020

Conversation

rbuffat
Copy link
Contributor

@rbuffat rbuffat commented Jul 5, 2020

Problem:
Some drivers create fields by default. Fiona is currently not aware of these fields. Additionally, some drivers modify the field names (e.g. Shapefile has a length restriction for field names). Fiona maintains a schema mapping between field names in the schema provided to Fiona, and the resulting field names. However, as the default fields are not considered, the resulting mapping is wrong (see #916).

This PR fixes the schema mapping if a driver contains default fields. This is kind of a minimal solution, as the existence of default fields also influence the schema validation, which is not touched with this PR. This PR does not modify that only the fields in the schema provided to Fiona are validated.

Currently, schema validation requires that records include all fields of the schema. However, drivers include also "optional" default fields, that do not need to be present. E.g. the default fields for the GPX driver are:

        'GPX': {'properties': OrderedDict([('ele', 'float'),
                                           ('time', 'datetime'),
                                           ('magvar', 'float'),
                                           ('geoidheight', 'float'),
                                           ('name', 'str'),
                                           ('cmt', 'str'),
                                           ('desc', 'str'),
                                           ('src', 'str'),
                                           ('link1_href', 'str'),
                                           ('link1_text', 'str'),
                                           ('link1_type', 'str'),
                                           ('link2_href', 'str'),
                                           ('link2_text', 'str'),
                                           ('link2_type', 'str'),
                                           ('sym', 'str'),
                                           ('type', 'str'),
                                           ('fix', 'str'),
                                           ('sat', 'int'),
                                           ('hdop', 'float'),
                                           ('vdop', 'float'),
                                           ('pdop', 'float'),
                                           ('ageofdgpsdata', 'float'),
                                           ('dgpsid', 'int')]),

E.g. thegeoidheight element is defined with minOccurs="0" in https://www.topografix.com/GPX/1/1/gpx.xsd

@coveralls
Copy link

coveralls commented Jul 5, 2020

Coverage Status

Coverage increased (+0.04%) to 84.294% when pulling d17f8db on rbuffat:default_fields into 01cfc24 on Toblerity:maint-1.8.

@sgillies
Copy link
Member

sgillies commented Aug 2, 2020

@rbuffat thank you! @snorfalorpagus thanks for the review!

@sgillies sgillies merged commit a07e6b7 into Toblerity:maint-1.8 Aug 2, 2020
@sgillies sgillies added this to the 1.8.14 milestone Aug 29, 2020
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.

4 participants