-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
CI: pre-commit fixups #40468
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
CI: pre-commit fixups #40468
Conversation
need to backport these? |
i think these both got introduced in the last day or so |
any experience with sqlalchemy? ive got a branch that is most of the way there, but their deprecation message
isnt very helpful in terms of telling me what context manager to use |
@@ -1372,9 +1372,9 @@ def array_likes(request): | |||
data = memoryview(arr) | |||
elif name == "array": | |||
# stdlib array | |||
from array import array | |||
from array import array as array_stdlib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MarcoGorelli I wonder is there a way to make the pre-commit check not trigger in cases like this (seems like this might be a false positive)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes...we can probably check that, if has been imported from pandas
and there is pd.
, then trigger, but don't if it was imported from somewhere else
I do think I prefer aliasing array
here if it's not from pandas, but I'll look into this nonetheless, thanks for bringing it up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I would say we should do import array
/ array.array(...)
. That should be clear?
good to go here? after this, #40471 should get us to green |
@MarcoGorelli ok for a followup if you want to check / rename the import |
Fixes 2/3 of the failures on master that im aware of. sqlalchemy is the one left out.