DEPR: Positional arguments in to_sql except name #54397
Conversation
rmhowe425
changed the title
|
@mroeschke pinging on green |
|
@mroeschke pinging on green |
mroeschke
changed the title
|
Thanks @rmhowe425 |
* Updated method header and whatsnew file * Updated unit tests to use keyword argument for con parameter. * Updating unit tests and implementation. * Updated documentation and unit tests. * Updating documentation and fixing unit tests. * Updating documentation. * Updating documentation and fixing failing unit tests. * Updating documentation and unit tests. * Updating implementation based on reviewer feedback. * Updating implementation to allow 'self' to be a positional arg. * Deprecating con positional arg in new test case. * Fixing typo * Fixing typo
| @deprecate_nonkeyword_arguments( | ||
| version="3.0", allowed_args=["self", "name"], name="to_sql" | ||
| ) | ||
| def to_sql( |
There was a problem hiding this comment.
It might make sense to allow con as positional as well?
(certainly given that it is a required argument, quite self-descriptive (eg not a boolean argument), and that this pattern is widely used in our own docs)
There was a problem hiding this comment.
Yeah I can see that being reasonable. I don't have super strong feelings about it though
There was a problem hiding this comment.
Let me know if I need to update the PR!
There was a problem hiding this comment.
I agree with @jorisvandenbossche, con is a weird name, I'd allow this to be positional as well (personally never specified it). Opened #54749 and PR is incoming, this should go into 2.1
| df = DataFrame([{"A": 1, "B": 2, "C": 3}, {"A": 1, "B": 2, "C": 3}]) | ||
|
|
||
| with tm.assert_produces_warning(FutureWarning, match=msg): | ||
| df.to_sql("example", self.conn) |
There was a problem hiding this comment.
should this use e.g. tm.ensure_clean? i think this may be leaving behind an "example" file after local test runs
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.