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

Modernize the codebase, reformat and improve performance #111

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

bswck
Copy link

@bswck bswck commented Nov 23, 2023

This PR contributes to #110 and features various improvements, including:

  • Modernize.
    • Use only new-style class declarations class T(object):class T:.
    • Use types from the built-in scope, not the deprecated ones like typing.List, typing.Dict etc. See PEP 585 and typing docs on built-in types aliases.
    • Postpone type annotations evaluation in runtime. See PEP 563.
    • Use PEP 604 X | Y union type syntax.
    • Use PEP 613 explicit type aliases.
    • Use typing.TYPE_CHECKING to separate typing-related imports from the actual runtime requirements.
    • Consistently use a zero-argument form of super() inside methods.
    • Use pathlib. Use Path(...).read_(text|bytes)() and Path(...).write_(text|bytes)() instead of the open() context manager where possible.
    • Use f"{i:x}" instead of hex(i)[2:]. Act analogically for other numeral systems. More information here.
  • Reformat.
    • Split all whole-line-width parameter lists into multi-line signatures with trailing commas.
    • Split one-line expressions that exceed max line length to multi-line.
    • Consistently use double-quote strings (advised and default format from black).
  • Improve performance.
    • Replace list/dict/set comprehensions/generator expressions with map/filter where applicable.
    • Prefer on-demand tuples to on-demand lists/sets.
    • Use itertools where applicable. Don't iterate over data passed to functions that accept iterables.
    • Use type(X) instead of X.__class__ where the __class__'s descriptor behavior doesn't matter.
    • Don't catch StopIteration when using next(), simply pass a default value as the other argument.
    • Don't call exception constructor when raising it with no message.

Note

To inspect the changes more easily, you might review every commit one-by-one chronologically.

All tests pass on my end.

Changes overview:
- Use types from the built-in scope, not the deprecated ones like `typing.List`, `typing.Dict` etc. See [PEP 585](https://peps.python.org/pep-0585/) and [typing docs on built-in types aliases](https://docs.python.org/3/library/typing.html#aliases-to-built-in-types).
- Postpone type annotations evaluation in runtime. See [PEP 563](https://peps.python.org/pep-0563/).
- Use [PEP 604](https://peps.python.org/pep-0604/) `X | Y` union type syntax.
- Use [PEP 613](https://peps.python.org/pep-0613/) explicit type aliases.
- Use `typing.TYPE_CHECKING` to separate typing-related imports from the actual runtime requirements.
Via `isort .`
@bswck
Copy link
Author

bswck commented Nov 23, 2023

During the review, a special attention should be paid to statements not covered by tests.

Coverage report before changes: https://smokeshow.helpmanual.io/3u0g2z34522g3a2k6k4u/
Coverage report after changes: https://smokeshow.helpmanual.io/640r041r4j1m2a2y6p1e/

@bswck
Copy link
Author

bswck commented Jan 15, 2024

@psrok1 I made this PR before a job interview @ NASK (unsuccessful). I'm curious about your feedback though and whether I should continue with the refactor. It inspired me to invent https://github.com/bswck/autorefine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant