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

API: SQL legacy mode to_sql 'index' kwarg behaviour #6881

Closed
jorisvandenbossche opened this issue Apr 14, 2014 · 3 comments · Fixed by #6883
Closed

API: SQL legacy mode to_sql 'index' kwarg behaviour #6881

jorisvandenbossche opened this issue Apr 14, 2014 · 3 comments · Fixed by #6883
Labels
IO SQL to_sql, read_sql, read_sql_query

Comments

@jorisvandenbossche
Copy link
Member

A leftover from #6735. In this PR, multi-index support was added to the new to_sql and read_table functions based on sqlalchemy. However, I did not change anything in the legacy to_sql functions.

This has the following consequences for the index handling in legacy mode (https://github.com/pydata/pandas/blob/18bd0d64bf1fcdc7e86e743332dab29e9a155909/pandas/io/sql.py#L808):

  • no multi-index support: so depending on the con type (dbapi connection or sqlalchemy connection), writing a multi-index dataframe will work or generate an error.
  • before, in 0.13.1 and before, there was actually no support for writing the index (it was just not written), so this is actually an API change for the legacy mode (because now writing the index is set to True by default), for write_frame (as to_sql did not yet exist)

We could also opt to remove this entirely from the legacy mode (leave it as it was). However this is also somewhat complicated, as it is not easy to detect when the index keyword is specified by the user in legacy mode (in order to warn that this is ignored), as it is set to True by default. But it seems to me that we should either support it fully (with multi-index as for sqlalchemy based), or not.

But maybe more in general: how do we see the 'legacy'? Just keep it for backwards compatibility? Or is it useful to have something that is not dependent on sqlalchemy? (so also enhance it? or only bug fixes?)

@hayd @mangecoeur @danielballan

@jorisvandenbossche jorisvandenbossche changed the title API: SQL legacy mode to_sql 'index' kwarg behaviour API: SQL legacy mode to_sql 'index' kwarg behaviour Apr 14, 2014
@danielballan
Copy link
Contributor

How do we see the 'legacy'?

In the long term, I'd like to keep full-fledged support for connections generated by sqlite3. For light applications, I still favor those over sqlalchemy for code portability (e.g., to collaborators who might not have sqlaclemy). Since all other flavors already require dependencies that are not built in to Python, I think it is reasonable to expect users to also obtain sqlalchemy if they want the very best support. One man's opinion.

So, I'd like to build MultiIndex support for legacy connections.

@jorisvandenbossche
Copy link
Member Author

Hmm, that's maybe indeed a good division. This would mean to deprecate the mysql flavor, and only keeping the slqite flavor.

I think it wouldn't be that difficult to add multi-index support to the legacy mode (actually a lot of the code can be reused I think. I will look at it).

jorisvandenbossche added a commit to jorisvandenbossche/pandas that referenced this issue Apr 14, 2014
and at once also index_label kwarg support
@jorisvandenbossche
Copy link
Member Author

@danielballan Pushed a PR for multi-index (and index_label specifying) support in legacy mode.

This just leaves the issue that this is actually an api change, so could possibly break code.

jeffreystarr pushed a commit to jeffreystarr/pandas that referenced this issue Apr 28, 2014
and at once also index_label kwarg support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IO SQL to_sql, read_sql, read_sql_query
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants