ENH: Added schema kwarg to get_schema method #33278
Conversation
gregorylivschitz
force-pushed
the
issue_28486
branch
from
8f8a60b to
6f8baf2
Compare
gregorylivschitz
changed the title
|
Please can you merge master |
gregorylivschitz
force-pushed
the
issue_28486
branch
from
6f8baf2 to
4552ca6
Compare
|
Seems like there are some test failures - mind taking a look? |
gregorylivschitz
force-pushed
the
issue_28486
branch
from
4552ca6 to
59e756a
Compare
|
I'm not sure what the issue was, it was not my test that was breaking, I rebased from master again and it seems to be working now. |
gregorylivschitz
force-pushed
the
issue_28486
branch
from
59e756a to
3e62959
Compare
|
@alimcmaster1 |
|
This looks good to me @gregorylivschitz mind merging master? |
|
@gregorylivschitz can you merge master |
|
@jreback |
|
@gregorylivschitz could you please merge master again? This looks like it's close to being merged |
|
@gregorylivschitz whatsnew entry will now be v1.2 Almost there |
|
I have merged it in. Thanks for reminding me! |
pandas/io/sql.py
Outdated
| @@ -1588,9 +1594,13 @@ def _create_table_setup(self): | |||
| create_tbl_stmts.append( | |||
| f"CONSTRAINT {self.name}_pk PRIMARY KEY ({cnames_br})" | |||
| ) | |||
|
|
|||
| if self.schema: | |||
| schema_to_add = self.schema + "." | |||
There was a problem hiding this comment.
Granted I'm no SQL expert, but perhaps schema_name would be a better variable name?
| @@ -866,6 +866,12 @@ def test_get_schema(self): | |||
| create_sql = sql.get_schema(self.test_frame1, "test", con=self.conn) | |||
| assert "CREATE" in create_sql | |||
|
|
|||
| def test_get_schema_with_schema(self): | |||
There was a problem hiding this comment.
can you add the github issue number here, e.g. # GH8624 (but with the correct number)
There was a problem hiding this comment.
@gregorylivschitz overall looks good to me, have just left some minor comments
pandas/io/sql.py
Outdated
| @@ -1455,9 +1455,15 @@ def drop_table(self, table_name, schema=None): | |||
| self.get_table(table_name, schema).drop() | |||
| self.meta.clear() | |||
|
|
|||
| def _create_sql_schema(self, frame, table_name, keys=None, dtype=None): | |||
| def _create_sql_schema(self, frame, table_name, keys=None, dtype=None, schema=None): | |||
There was a problem hiding this comment.
While you're here, could you add type annotations to these arguments? Not all old code has them, but they're encouraged for new developments. Feel free to ping if it's not something you're familiar with
|
Hello @gregorylivschitz! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-08-28 03:32:20 UTC |
|
@gregorylivschitz I think this looks good but has a merge conflict and a CI failure - can you take a look? Should be able to get this in soon |
There was a problem hiding this comment.
@gregorylivschitz if you merge master and resolve conflicts it sounds like this is good to go in
|
Closing in favor of #38146 |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff