ENH: Support index=True for io.sql.get_schema #25030
Conversation
Codecov Report
@@ Coverage Diff @@
## master #25030 +/- ##
=======================================
Coverage 92.38% 92.38%
=======================================
Files 166 166
Lines 52401 52401
=======================================
Hits 48409 48409
Misses 3992 3992
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #25030 +/- ##
==========================================
- Coverage 92.37% 92.36% -0.01%
==========================================
Files 166 166
Lines 52403 52403
==========================================
- Hits 48405 48404 -1
- Misses 3998 3999 +1
Continue to review full report at Codecov.
|
doc/source/whatsnew/v0.24.1.rst
Outdated
| ``get_schema`` Enhancements | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| :func:`get_schema` now accepts an `index` parameter (default: `False`) that includes the index in the generated schema. (:issue:`9084`) |
There was a problem hiding this comment.
Move this to the 0.25.0 whatsnew.
There was a problem hiding this comment.
I would also just add it as a bullet point under the I/O section of the document.
pandas/io/sql.py
Outdated
| @@ -1588,8 +1590,11 @@ def get_schema(frame, name, keys=None, con=None, dtype=None): | |||
| dtype : dict of column name to SQL type, default None | |||
| Optional specifying the datatype for columns. The SQL type should | |||
| be a SQLAlchemy type, or a string for sqlite3 fallback connection. | |||
| index : boolean, default False | |||
| include DataFrame index as a column | |||
There was a problem hiding this comment.
"include..." --> "Whether to include..."
Closes pandas-dev#9084 - Decided to keep the default as `index=False` to keep the API consistent. `to_sql` has `index=True`. - Tempted to name the parameter `include_dataframe_index` as "index" has a different meaning in a SQL context.
vrajmohan
force-pushed
the
include-index-in-get-schema
branch
from
5cf0e3e
to
722dc56
Compare
| @@ -1589,8 +1591,11 @@ def get_schema(frame, name, keys=None, con=None, dtype=None): | |||
| dtype : dict of column name to SQL type, default None | |||
| Optional specifying the datatype for columns. The SQL type should | |||
| be a SQLAlchemy type, or a string for sqlite3 fallback connection. | |||
| index : boolean, default False | |||
| Whether to include DataFrame index as a column | |||
There was a problem hiding this comment.
Can you add a period here at the end of the description?
| @@ -31,7 +31,6 @@ Fixed Regressions | |||
| Enhancements | |||
| ^^^^^^^^^^^^ | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Can you revert changes to this file?
| @@ -823,6 +823,21 @@ def test_get_schema_keys(self): | |||
| constraint_sentence = 'CONSTRAINT test_pk PRIMARY KEY ("A", "B")' | |||
| assert constraint_sentence in create_sql | |||
|
|
|||
| @pytest.mark.parametrize("index_arg, expected", [ | |||
There was a problem hiding this comment.
I think it would be better if you simply parametrized on True and False and made "expected" the actual expected statement
| ({"index": True}, True), | ||
| ]) | ||
| def test_get_schema_with_index(self, index_arg, expected): | ||
| frame = DataFrame({ |
|
I think this has gone stale but please ping if you'd like to continue! |
Closes #9084
Decided to keep the default as
index=Falseto keep the API consistent.to_sqlhasindex=True.Tempted to name the parameter
include_dataframe_indexas "index" hasa different meaning in a SQL context.
passes
git diff upstream/master -u -- "*.py" | flake8 --diffwhatsnew entry