chore: harmonize python enum naming conventions #26948
Conversation
There was a problem hiding this comment.
@villebro it would be great if pylint could prevent any future regressions. Per pylint-dev/pylint#3896 it seems like this functionality exists, so I'm a little perplexed as to why it's not working.
|
Thanks for the pointing me in the right direction @john-bodley! I agree, this seems to be covered by vanilla |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #26948 +/- ##
==========================================
- Coverage 69.50% 59.66% -9.85%
==========================================
Files 1900 1899 -1
Lines 74347 73976 -371
Branches 8264 8041 -223
==========================================
- Hits 51677 44138 -7539
- Misses 20621 27869 +7248
+ Partials 2049 1969 -80
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| object_id=1, object_type=ObjectType.dashboard.name, tag_id=tag.id | ||
| object_id=1, object_type=ObjectType.DASHBOARD, tag_id=tag.id |
There was a problem hiding this comment.
@john-bodley the fact that my IDE was able to identify these type errors indicates that our /tests/ directory is in need of some serious linting love. I think the time has come to enable full linting on our Python test suites. WDYT? I can't remember how many bycatch unused imports I've removed over the last few years. I can probably start by cleaning up the unit test suite, as that seems to be the simplest place to start..
There was a problem hiding this comment.
@villebro I’ve been making some rather XXL refactors recently and wished that we had mypy and pylint checks for the entirety of the Python code base including tests and migrations.
There was a problem hiding this comment.
I've found mypy can be very temperamental, but mostly that's in terms of checking far beyond the scope of what it's being asked to in the first place. It does do a good job of catching problems when it's not a false positive though. pylint is much less temperamental so at least having that seems a no-brainer.
villebro
force-pushed
the
villebro/py-enum
branch
from
befa87a
to
0c9978d
Compare
|
michael-s-molina
added
the
v4.0
|
This turned ugly, just as I suspected 🤦 Link to a Slack thread that I opened to get some feedback: https://apache-superset.slack.com/archives/CCKHMGRRB/p1706829726367459?thread_ts=1706828839.332729&cid=CCKHMGRRB |
michael-s-molina
added
v4.0
michael-s-molina
added
v5.0
SUMMARY
This is a continuation of #26875, and refactors all Python enums to comply with the commonly used naming conventions of PascalCase for the class name, and UPPER_CASE for the names: https://docs.python.org/3/library/enum.html This exercise was quite a bit less painful than on the frontend. 😸
Unfortunately I'm not aware of anything that would come close to ESLint for Python, so for now we'll just have to try to make sure everyone adheres to these conventions during code review. I believe something similar might be possible with pylint, but I wasn't able to scope it to only apply to enums, so I'll just leave it be. But if someone knows how this can be done, or can point me in the right direction, please do let me know.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION