Skip to content

CLN: sql static method #36410

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.

Sign up for GitHub

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

Closed
wants to merge 42 commits into from
Closed

CLN: sql static method #36410

Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
5bbddce
DEPR - 18262 - deprecate lookup
erfannariman Jul 11, 2020
02b50ac
DEPR - 18262 - changes black
erfannariman Jul 11, 2020
ca8bf05
Add test for deprecation lookup
erfannariman Jul 11, 2020
dea45b9
Added deprecation to whatsnew
erfannariman Jul 11, 2020
5f116ad
Add test for deprecation lookup
erfannariman Jul 11, 2020
0c04e90
FIX - 18262 - add warnings to other tests
erfannariman Jul 11, 2020
758468d
Merge remote-tracking branch 'upstream/master' into 18262-depr-lookup
erfannariman Jul 11, 2020
7ac3b32
DOC - 18262 - added example to lookup values
erfannariman Jul 11, 2020
2ab80cf
DOC - 18262 - point to example in depr
erfannariman Jul 11, 2020
94a6c0f
FIX - 18262 - deprecation warning before summary
erfannariman Jul 11, 2020
a339131
FIX - 18262 - whitespaces after comma
erfannariman Jul 11, 2020
269e4cc
FIX - 18262 - removed double line break
erfannariman Jul 11, 2020
0c40d69
Fix variable in ipython
erfannariman Jul 11, 2020
1ca23bc
18262 - removed linebreak
erfannariman Jul 12, 2020
9681a3d
18262 - Fix merge conflict
erfannariman Jul 15, 2020
6342ad2
18262 - replaced depr message
erfannariman Jul 15, 2020
3dfe19d
18262 - line break too long line
erfannariman Jul 15, 2020
187d47b
18262 - set header size
erfannariman Jul 15, 2020
db63df7
[FIX] - 18262 - Merge conflict
erfannariman Jul 28, 2020
227fad5
[FIX] - 18262 - removed extra dash header
erfannariman Jul 28, 2020
ce775ce
[FIX] - 18262 - reference to section in docs
erfannariman Jul 28, 2020
2ee7d09
[FIX] - 18262 - grammar / typos in docstring
erfannariman Jul 28, 2020
4c78311
Merge branch 'master' into 18262-depr-lookup
erfannariman Jul 28, 2020
f447185
Merge branch 'master' into 18262-depr-lookup
erfannariman Sep 13, 2020
dc2d367
moved depr version to 1.2
erfannariman Sep 13, 2020
293bd7a
test with linking to user guide
erfannariman Sep 13, 2020
cbca163
Remove line break
erfannariman Sep 13, 2020
90fa6a9
Merge branch 'master' into 18262-depr-lookup
erfannariman Sep 13, 2020
3eefd8e
Merge branch 'master' into 18262-depr-lookup
erfannariman Sep 14, 2020
4c3c163
Revert whatsnew v1.1.0
erfannariman Sep 14, 2020
b5a34e3
Added depr message in whatsnew v1.2.0
erfannariman Sep 14, 2020
ba4fb8a
replace query with loc
erfannariman Sep 14, 2020
6b91db6
add melt and loc to depr msg
erfannariman Sep 14, 2020
ff7724f
add dot
erfannariman Sep 14, 2020
104e3cb
added colon hyperlink
erfannariman Sep 15, 2020
25e78dd
updates
erfannariman Sep 16, 2020
32affb2
Merge branch 'master' into 18262-depr-lookup
erfannariman Sep 16, 2020
4f4bf65
static method
erfannariman Sep 16, 2020
45bdc91
more clean up in sql.py
erfannariman Sep 16, 2020
d5f13b7
revert removal
erfannariman Sep 17, 2020
381e203
remove self
erfannariman Sep 17, 2020
33a47d1
Merge branch 'master' into cln-sql-static-method
erfannariman Sep 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions pandas/io/sql.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1079,7 +1079,8 @@ def _sqlalchemy_type(self, col):

return Text

def _get_dtype(self, sqltype):
@staticmethod
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1 on static methods generally; you can move to a free function if you really want

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree we should generally be using staticmethods only sparingly, but IMO here that is fine. This method is only used once, just above, and keeping it here keeps it closer to where it is used.
The same for _fetchall_as_list. And the get_table is actually overriding a parent class method, so shouldn't be moved to a free function.

So I prefer to keep those functions where they are. Whether they are normal methods or staticmethods, I personally don't care. So if others don't want to use staticmethods, I would just leave this as is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess with the -1 and with the leave as is, I will close this PR.

def _get_dtype(sqltype):
from sqlalchemy.types import TIMESTAMP, Boolean, Date, DateTime, Float, Integer

if isinstance(sqltype, Float):
Expand Down Expand Up @@ -1750,7 +1751,8 @@ def read_query(
)
return frame

def _fetchall_as_list(self, cur):
@staticmethod
def _fetchall_as_list(cur):
result = cur.fetchall()
if not isinstance(result, list):
result = list(result)
Expand Down Expand Up @@ -1838,7 +1840,8 @@ def has_table(self, name, schema=None):

return len(self.execute(query, [name]).fetchall()) > 0

def get_table(self, table_name, schema=None):
@staticmethod
def get_table(table_name, schema=None):
return None # not supported in fallback mode

def drop_table(self, name, schema=None):
Expand Down