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

SQLite Result ValueDateTime regression to non-datetime type #158

Closed
emiliom opened this issue Sep 23, 2018 · 1 comment
Closed

SQLite Result ValueDateTime regression to non-datetime type #158

emiliom opened this issue Sep 23, 2018 · 1 comment

Comments

@emiliom
Copy link
Member

emiliom commented Sep 23, 2018

getResultValues on a measurement-type SQLite ODM2 database is now returning ValueDateTime types as strings rather than datetime.

This was not the case last November, when the same code with the same database returned datetime types. In this notebook from the BiGCZ workshop in early-mid November 2017, the SQLite database is examined extensively. The time series plots in cells 24 and 26 correctly show valuedatetime as a datetime. Running that code today with a conda environment created back then, with odm2api 0.6.0a (created May 2017), also produces the expected type. But running it with the current odm2api release (0.7.1) returns a unicode-string-type valuedatetime.

I've traced back changes that could have led to this regression (and bug). I think I've found it in this commit from 2017-11-13 ("update tests, update DateTime data type to be compatible with sqlite", merged in PR #122), in its changes to models.py. Specifically:
8a8ffe7#diff-0ee3d0270e8061c9eb0e712c2e83d27aR18 and 8a8ffe7#diff-0ee3d0270e8061c9eb0e712c2e83d27aR1658 (as well as all other ResultValues classes)

I'm hoping the fix will be simple, involving changing https://github.com/ODM2/ODM2PythonAPI/blob/master/odm2api/models.py#L17 from

DateTimeType = DateTimeType.with_variant(sqlite.INTEGER(), 'sqlite')

to

DateTimeType = DateTimeType.with_variant(sqlite.DATETIME(), 'sqlite')

But I haven't tested it yet.

@emiliom
Copy link
Member Author

emiliom commented Apr 20, 2019

Fixed by #169. Closing

@emiliom emiliom closed this as completed Apr 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant