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

Test support for date/datetime/time fields #563

Merged
merged 3 commits into from
Jun 10, 2018

Conversation

snorfalorpagus
Copy link
Member

#540 got me looking at the support for date/datetime/time fields. It's a little complicated, with different drivers having different levels of support and behaviours. I've added some tests to illustrate the current behaviour. I don't think anything needs to change, except that we should raise a more useful error when trying to create a time-type field in an ESRI Shapefile, which currently raises an unexpected KeyError.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 84.065% when pulling 8acc147 on snorfalorpagus:datetime_tests into 35bdccb on Toblerity:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 84.065% when pulling 8acc147 on snorfalorpagus:datetime_tests into 35bdccb on Toblerity:master.

@coveralls
Copy link

coveralls commented Mar 25, 2018

Coverage Status

Coverage increased (+0.3%) to 84.07% when pulling e2b84b4 on snorfalorpagus:datetime_tests into f3ce335 on Toblerity:master.

@snorfalorpagus
Copy link
Member Author

Apparently GPKG support for datetime is in GDAL 2.x only. Need to update test further.

@snorfalorpagus
Copy link
Member Author

This is an absolute rabbit hole! There are more exceptions than rules, with different behaviour between GDAL 1/2 and different formats. I think we should be raising warnings for some of these differences (silent type conversions, dropping times from dates, etc). I think the test needs reformatting also as with all the exceptions it's spaghetti. I'll keep working on this.

@sgillies
Copy link
Member

sgillies commented Jun 6, 2018

@snorfalorpagus sorry for the review backlog! I'll try to get to them tomorrow. I see that we have an unresolved dependency on Windows.

@snorfalorpagus
Copy link
Member Author

snorfalorpagus commented Jun 6, 2018

sorry for the review backlog! I'll try to get to them tomorrow. I see that we have an unresolved dependency on Windows.

No problem! I created #593 for the dependency issue.

The approach I've taken here is to raise a DriverSupportError if GDAL would do something unexpected (e.g. writing a date when a datetime was asked for), or a warning if the output is only mildly unusual (e.g. a non-standard date format).

@sgillies
Copy link
Member

sgillies commented Jun 8, 2018

Wow, I'm baffled by the failure on Appveyor. The click-plugins project hasn't changed in a long time and we used to pass the tests with pip 10.0.1. I'm stumped for now.

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.

3 participants