chore(pre-commit): Add pyupgrade and pycln hooks #24197

john-bodley · 2023-05-24T08:02:08Z

SUMMARY

I thought there was merit in "keeping up with the Joneses" in terms of ensuring our Python code is inline with the evolution of the language.

pyupgrade is a tool (and pre-commit hook) to automatically upgrade syntax for newer versions of the language. This PR adds said pre-commit hook (in addition to pycln which removes unused imports) to help ensure your code style format is fresh.

Apologies for the scale of the change. I wasn't able to find any setting in pyupgrade to make this more digestible, unless we opted to incrementally set the minimum Python requirement.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

CI.

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

superset/views/core.py

        )
        if has_access_:
-            return redirect("/superset/dashboard/{}".format(dashboard_id))
+            return redirect(f"/superset/dashboard/{dashboard_id}")


john-bodley · 2023-05-24T17:14:19Z

.pre-commit-config.yaml

+        args:
+          - --keep-runtime-typing
+          - --py38-plus
+  - repo: https://github.com/hadialqattan/pycln


pyupgrade may perform type rewrites, etc. rendering some imports obsolete. It seemed prudent to also include a hook which then cleans up unused imports.

Does pyupgrade is restrained to our supported Python versions? Another way of asking is if a new Python version is launched with a new syntax, will pyupgrade try to change the files to the new syntax before we support this new version?

@michael-s-molina yes. Per the --py39-plus CLI argument it ensures that the changes are compatible with Python versions >= 3.9 which is the minimum version we currently support.

codecov · 2023-05-24T17:35:25Z

Codecov Report

Merging #24197 (e7a2fe1) into master (f6e769a) will increase coverage by 0.00%.
The diff coverage is 93.71%.

❗ Current head e7a2fe1 differs from pull request most recent head b1cda7c. Consider uploading reports for the commit b1cda7c to get more accurate results

@@           Coverage Diff           @@
##           master   #24197   +/-   ##
=======================================
  Coverage   68.31%   68.31%           
=======================================
  Files        1957     1957           
  Lines       75597    75621   +24     
  Branches     8222     8222           
=======================================
+ Hits        51641    51661   +20     
- Misses      21848    21852    +4     
  Partials     2108     2108

Flag	Coverage Δ
hive	`53.39% <81.21%> (+0.02%)`	⬆️
mysql	`78.92% <92.81%> (+<0.01%)`	⬆️
postgres	`79.00% <92.88%> (+<0.01%)`	⬆️
presto	`53.32% <81.21%> (+0.02%)`	⬆️
python	`82.82% <93.71%> (+<0.01%)`	⬆️
sqlite	`77.53% <92.29%> (+<0.01%)`	⬆️
unit	`53.44% <80.38%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset/cli/main.py	`0.00% <0.00%> (ø)`
superset/cli/native_filters.py	`0.00% <0.00%> (ø)`
superset/commands/importers/v1/examples.py	`0.00% <0.00%> (ø)`
superset/common/tags.py	`0.00% <0.00%> (ø)`
superset/connectors/sqla/views.py	`89.71% <0.00%> (ø)`
superset/examples/bart_lines.py	`0.00% <0.00%> (ø)`
superset/examples/big_data.py	`0.00% <0.00%> (ø)`
superset/examples/countries.py	`0.00% <0.00%> (ø)`
superset/examples/multiformat_time_series.py	`0.00% <0.00%> (ø)`
superset/examples/paris.py	`0.00% <0.00%> (ø)`
... and 260 more

... and 11 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

john-bodley · 2023-05-24T17:55:55Z

.pre-commit-config.yaml

+      - id: pycln
+        args:
+          - --disable-all-dunder-policy
+          - --exclude=superset/config.py


pycln chokes on module not found errors when we try importing the default configs.

john-bodley · 2023-05-25T18:27:39Z

superset/connectors/base/models.py

-        db_engine_spec: Optional[Type[BaseEngineSpec]] = None,
-        db_extra: Optional[Dict[str, Any]] = None,
-    ) -> Optional[FilterValues]:
+        db_engine_spec: builtins.type[BaseEngineSpec] | None = None,


See .python/mypy#15047 (comment).

michael-s-molina

LGTM. Thank you very much for this improvement @john-bodley! It will contribute a lot to code standardization.

Reviewing this PR made me aware of how much untyped structures (dict) we have in the codebase and the consequences of that in terms of bugs and duplicated logic 😟

I'm also going to ping @villebro @eschutho @jfrag1 in case they have extensions that may be affected by this PR.

michael-s-molina · 2023-06-01T17:54:46Z

superset/connectors/sqla/models.py

-        self, template_processor: Optional[BaseTemplateProcessor] = None
-    ) -> Tuple[Union[TableClause, Alias], Optional[str]]:
+        self, template_processor: BaseTemplateProcessor | None = None
+    ) -> tuple[TableClause | Alias, str | None]:


It's way more readable then the Optional version 😄

Yup. The term Optional is such a misnomer and a bad choice to represent something that could be None. The PEP-604 typing rewrites currently only apply to those files which contain the following,

from __future__ import annotations

though will become ubiquitous after we drop support for Python 3.9.

john-bodley force-pushed the john-bodley--pre-commit-hook-pyupgrade branch from 3573145 to e92b2d3 Compare May 24, 2023 08:02

pull-request-size bot added the size/XL label May 24, 2023

github-advanced-security bot found potential problems May 24, 2023

View reviewed changes

john-bodley changed the title ~~feature(pre-commit): Keeping up with the Joneses~~ chore(python): Keeping up with the Joneses May 24, 2023

john-bodley changed the title ~~chore(python): Keeping up with the Joneses~~ chore: Keeping up with the Joneses - Python syntax May 24, 2023

pull-request-size bot added size/XXL and removed size/XL labels May 24, 2023

john-bodley changed the title ~~chore: Keeping up with the Joneses - Python syntax~~ chore(python): Keeping up with the Joneses May 24, 2023

john-bodley force-pushed the john-bodley--pre-commit-hook-pyupgrade branch from 2328dc8 to e92b2d3 Compare May 24, 2023 16:39

pull-request-size bot added size/XL and removed size/XXL labels May 24, 2023

john-bodley changed the title ~~chore(python): Keeping up with the Joneses~~ chore(pyupgrade): Keeping up with the Joneses May 24, 2023

john-bodley commented May 24, 2023

View reviewed changes

john-bodley marked this pull request as ready for review May 24, 2023 17:23

john-bodley requested a review from a team as a code owner May 24, 2023 17:23

john-bodley force-pushed the john-bodley--pre-commit-hook-pyupgrade branch from e92b2d3 to 2af3468 Compare May 24, 2023 17:54

john-bodley commented May 24, 2023

View reviewed changes

john-bodley requested review from betodealmeida, eschutho, villebro and michael-s-molina May 24, 2023 18:33

john-bodley changed the title ~~chore(pyupgrade): Keeping up with the Joneses~~ chore(pre-comit): Add pyupgrade and pycln hooks May 24, 2023

john-bodley changed the title ~~chore(pre-comit): Add pyupgrade and pycln hooks~~ chore(pre-commit): Add pyupgrade and pycln hooks May 24, 2023

john-bodley force-pushed the john-bodley--pre-commit-hook-pyupgrade branch 2 times, most recently from cfab874 to 8952492 Compare May 25, 2023 18:26

john-bodley commented May 25, 2023

View reviewed changes

john-bodley force-pushed the john-bodley--pre-commit-hook-pyupgrade branch 2 times, most recently from b8b64fa to 9e99d8c Compare May 25, 2023 19:02

michael-s-molina approved these changes Jun 1, 2023

View reviewed changes

feature(pre-commit): Keeping up with the Joneses

b1cda7c

john-bodley force-pushed the john-bodley--pre-commit-hook-pyupgrade branch from 9e99d8c to b1cda7c Compare June 1, 2023 18:21

john-bodley merged commit a4d5d7c into apache:master Jun 1, 2023
29 checks passed

john-bodley deleted the john-bodley--pre-commit-hook-pyupgrade branch June 1, 2023 19:01

sfirke mentioned this pull request Sep 6, 2023

docs: fix wrong type in PREFERRED_DATABASES example #25200

Merged

9 tasks

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.0.0 labels Mar 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(pre-commit): Add pyupgrade and pycln hooks #24197

chore(pre-commit): Add pyupgrade and pycln hooks #24197

john-bodley commented May 24, 2023 •

edited

Loading

john-bodley May 24, 2023

michael-s-molina May 31, 2023

john-bodley Jun 1, 2023

codecov bot commented May 24, 2023 •

edited

Loading

john-bodley May 24, 2023

john-bodley May 25, 2023

michael-s-molina left a comment

michael-s-molina Jun 1, 2023

john-bodley Jun 1, 2023

chore(pre-commit): Add pyupgrade and pycln hooks #24197

chore(pre-commit): Add pyupgrade and pycln hooks #24197

Conversation

john-bodley commented May 24, 2023 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

john-bodley May 24, 2023

Choose a reason for hiding this comment

michael-s-molina May 31, 2023

Choose a reason for hiding this comment

john-bodley Jun 1, 2023

Choose a reason for hiding this comment

codecov bot commented May 24, 2023 • edited Loading

Codecov Report

john-bodley May 24, 2023

Choose a reason for hiding this comment

john-bodley May 25, 2023

Choose a reason for hiding this comment

michael-s-molina left a comment

Choose a reason for hiding this comment

michael-s-molina Jun 1, 2023

Choose a reason for hiding this comment

john-bodley Jun 1, 2023

Choose a reason for hiding this comment

john-bodley commented May 24, 2023 •

edited

Loading

codecov bot commented May 24, 2023 •

edited

Loading