BUG: to_sql with datetime.time values with sqlite
State: closed

I'm proposing a solution to #8341

As @jorisvandenbossche suggested, I use a sqlite3 adapter to transform `datetime.time` objects into string (`hh:mm:ss.ffffff`, as this what sqlalchemy... I think?)

I added a test in the class `_TestSQLApi`, so that the solution is tested with both sqlalchemy and the sqlite3 fallback. Thus, the result should be consistent.


Labels:
name: IO SQL
description: to_sql, read_sql, read_sql_query

Comments:
@nbonnotte Thanks for working on this!

As a quick feedback: I think we will also want this to work in the other direction, so from sql -> python, that you get back `datetime.time` instances (so not only a 'adapter' but also 'converter' in sqlite terminology, https://docs.python.org/2/library/sqlite3.html#converting-sqlite-values-to-custom-python-types)

A quick update

@jorisvandenbossche to go the other way around is a bit more tricky, because (if i'm not mistaken) we depend on the sqlite3 connexion provided by the user.

Quick recap: in order for sqlite3 to know how to convert data back into the proper Python object, the user need to provide a parameter `detect_types` to `sqlite3.connect` specifying how to do that. There are 3 possibilities: 
1. The value `sqlite3.PARSE_DECLTYPES` tells sqlite3 to "parse" (how exactly?) the declared sql type
2. The value `sqlite3.PARSE_COLNAMES` tells it to use the column name
3. Not providing any value tells sqlite3 not to convert the data and let it as a string

Since the dataframe we want to sqlize most likely has names for its columns, I guess we can exclude option 2. For backward compatibility reason, option 3 needs to be kept working, and option 1 is the one we want to implement here.

From a very practical point of view, we thus need to have tests for both option 1 and option 3, because we don't know the connection the user will throw at us.

That's when some difficulties arise: in `pandas/io/test_sql.py`, just adding the option `detect_types=sqlite3.PARSE_DECLTYPES` in all `sqlite3.connect` without specifying any convertors causes errors for `time` and `date` objects. One is a bit surprising:

```
  File "/home/nicolas/Git/pandas/pandas/io/tests/test_sql.py", line 1923, in test_datetime_date
    tm.assert_frame_equal(res, df.astype(str))
...
DataFrame.iloc[:, 0] values are different (100.0 %)
[left]:  [2014-01-01, 2014-01-02]
[right]: [2014-01-01, 2014-01-02]
```

I hope it's just a matter of type conversion, that for some strange reason looks really really weird.

I haven't had much time to look precisely onto it yet, but going the other way around (sqlite → python) will definitely demand more work than my initial pull request (python → sqlite).

@jorisvandenbossche So basically, with the current pull request, the sqlite3 fallback should work exactly as with sqlalchemy. 

To go further, that is to also provide a converter, would cause the "sqlite3 fallback" to have more features than with sqlalchemy, because then you would be able with the fallback to end up with, say, `datetime` objects, but not with sqlalchemy. 

Am I wrong?

And assuming it is still an interesting direction to go, wouldn't it be better to have a new, separate issue, and let this PR close #8341?

@jorisvandenbossche status of this?

status of this?

@jorisvandenbossche Maybe we could let this PR close issue #8341, and start a new issue to go in the other direction (sql -> python) ? What do you think?

@nbonnotte yes, certainly a good idea to limit it here to pandas->sql!

Great. And it's all green! \o/

Pull request updated and green

@jorisvandenbossche ?

@nbonnotte Sorry for the delay, but some comments on the tests

I've updated the PR taking into account comments from @jorisvandenbossche 

Thanks @nbonnotte 
I you put the whatsnew in 0.18.0, I think this can still be merged

Ok. If it can be shipped in 0.18.0, all the better — otherwise I'll change the whatsnew again.

All green.

@nbonnotte merged, thanks!