-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
fix: Python3.11 (str, Enum) issue #24803
fix: Python3.11 (str, Enum) issue #24803
Conversation
Codecov Report
@@ Coverage Diff @@
## master #24803 +/- ##
==========================================
+ Coverage 68.95% 68.99% +0.03%
==========================================
Files 1902 1904 +2
Lines 73998 74081 +83
Branches 8195 8193 -2
==========================================
+ Hits 51029 51115 +86
+ Misses 20850 20845 -5
- Partials 2119 2121 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
b2e1f15
to
61e075e
Compare
61e075e
to
f9ff25c
Compare
Please look into 3 failing tests. |
This reverts commit f9ff25c.
|
.pylintrc
Outdated
@@ -75,6 +75,7 @@ enable= | |||
# --disable=W" | |||
disable= | |||
cyclic-import, # re-enable once this no longer raises false positives | |||
no-member, # re-enable once this no longer raises false positives |
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.
I mentioned the my concerns about blanket disabling of messages in the past. Is this a necessary requirement?
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, it's necessary requirement before we update min supported python to 3.11
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.
Maybe we could add a note in the comment that this will become redundant after the min required version is 3.11
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.
meanwhile raised the issue on pylint GitHub pylint-dev/pylint#8897
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.
@villebro Added the note in the comment
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.
LGTM, would love @john-bodley 's approval for merging
.pylintrc
Outdated
@@ -75,6 +75,7 @@ enable= | |||
# --disable=W" | |||
disable= | |||
cyclic-import, # re-enable once this no longer raises false positives | |||
no-member, # re-enable once this no longer raises false positives |
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.
Maybe we could add a note in the comment that this will become redundant after the min required version is 3.11
I think, we also need to update files where |
Thanks @mdeshmu! Updated it. |
Hey @john-bodley @michael-s-molina @rusackas! Can we merge this PR? |
SUMMARY
Changed Enum.format() (the default for format(), str.format() and f-strings) to always produce the same result as Enum.str(): for enums inheriting from ReprEnum it will be the member's value; for all other enums it will be the enum and member name (e.g. Color.RED).
Fix the next Py3.11 error
sqlalchemy.exc.ProgrammingError: (psycopg2.errors.SyntaxError) syntax error at or near "CtasMethod" LINE 1: DROP CtasMethod.TABLE IF EXISTS test_sync_table
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION