Skip to content
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

update style to more modern python idioms and remove dead imports #157

Merged
merged 8 commits into from
Aug 15, 2024

Conversation

aaraney
Copy link
Member

@aaraney aaraney commented Aug 6, 2024

This PR does introduce behavioral changes. Its highly recommended to view each commit individually during review. Ive split things up by the refactoring / stylistic operation that was performed to make this easier to review.

The refactors / stylistic improvements performed are:

chore:

  • remove unused imports
  • add from __future__ import annotations to relevant files

refactor:

  • guard non-runtime types with typing.TYPE_CHECKING
  • single member typing.Union

style:

  • use __future__ annotations and remove quotes from, now, non-runtime type hints
  • drop open's default 'r'
  • use f-strings

Note: non-typing types (e.g. list[str]) do not work with pydantic for python<=3.8. See this issue for more information.

@aaraney aaraney requested a review from hellkite500 August 6, 2024 20:22
@aaraney aaraney added ngen.config Related to ngen.config package ngen.cal Related to ngen.cal package ngen.init_config Related to ngen.init_config package ngen.config_gen Related to ngen.config_gen package labels Aug 6, 2024
@aaraney aaraney requested a review from robertbartel August 6, 2024 20:23
@aaraney aaraney force-pushed the style-updates branch 2 times, most recently from 53e8817 to 62370dc Compare August 6, 2024 21:05
@aaraney aaraney force-pushed the style-updates branch 4 times, most recently from 62370dc to 0189079 Compare August 14, 2024 17:41
@aaraney
Copy link
Member Author

aaraney commented Aug 14, 2024

@hellkite500, I think we should be good to merge this now.

hellkite500
hellkite500 previously approved these changes Aug 15, 2024
@hellkite500 hellkite500 merged commit 5ef42ca into NOAA-OWP:master Aug 15, 2024
9 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ngen.cal Related to ngen.cal package ngen.config_gen Related to ngen.config_gen package ngen.config Related to ngen.config package ngen.init_config Related to ngen.init_config package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants