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

Improve Monetary API, Various Chores #19

Merged
merged 28 commits into from
Jun 15, 2023
Merged

Improve Monetary API, Various Chores #19

merged 28 commits into from
Jun 15, 2023

Conversation

vst
Copy link
Owner

@vst vst commented Jun 14, 2023

Closes #18.

  • refactor: use PEP484-compatible type hint
  • fix: use more common names for pre-defined currencies
  • fix: correct spelling for some documentation and exception messages
  • chore: gitignore /.vscode directory
  • test: replace tox with nox
  • test: add pyright to our test suite
  • fix: fix Protocol definitions
  • fix: make NonePrice.money a property
  • test: silence pyright error for false positive
  • feat: add {Money,Price}.qty_or_zero method
  • feat: add {Money,Price}.qty_or_none method
  • feat: add {Money,Price}.qty_or_else method
  • feat: add {Money,Price}.qty_map method
  • feat: add {Money,Price}.fmap method
  • feat: add {Money,Price}.dimap method
  • feat: add {Money,Price}.ccy_or_none method
  • feat: add {Money,Price}.dov_or_none method
  • feat: add {Money,Price}.or_else method
  • test: enable pylint tests
  • refactor: attend pylint check errors (invalid-name)
  • refactor: attend various pylint check errors
  • refactor: remove unnecessary abstractmethod body
  • refactor: make Money and Price abstract base class
  • refactor: make {Money,Price}.{defined,undefined} properties
  • refactor!: make {Money,Price}.NA a classmethod, rename to .na
  • refactor!: drop {Money,Price}.{ccy,qty,dov} type hints
  • test: reconfigure pylint
  • feat: add type guards for {None,Some}{Money,Price} type checks

vst added 26 commits June 14, 2023 07:27
Instead of `pass` we use Python ellipsis (`...`) to make `pyright`
happy.
`pyright` keeps complaining about `money` not being a property. `mypy`
does not.

The rationale of `pyright` developers seem to be right:

microsoft/pyright#2601 (comment)
Looks like min/max and iterable of iterables cause a false positive
with pyright. This is most likely due to that `pyright` is picking the
wrong overloaded function signature.
Initially, we are disabling a bunch of checks. We will enable them as
we fix them.
Fixed:

- too-many-lines
- redefined-builtin
- line-too-long
- consider-using-in

Disabled:

- duplicate-code
For safety and style purposes, `{Money,Price}.NA` is renamed to
`{Money,Price}.na` and made a classmethod.
This is not really affecting the runtime behaviour. However, type
checking and intellisense will complain about that these properties
may not exist on Money/Price object.

For call-sites: Type guards or `isinstance` might work well to "cast"
to some/none money/price instance.
@vst vst linked an issue Jun 14, 2023 that may be closed by this pull request
17 tasks
@vst vst self-assigned this Jun 14, 2023
@vst vst requested a review from fkoksal June 14, 2023 14:00
vst added 2 commits June 15, 2023 07:45
- We want to locally disable some errors.
- Disable "suppressed-message" as we may disable some errors.
- Uses "PEP 647 – User-Defined Type Guards"
- Depends on `typing_extensions` for Python versions <3.10
- Increased pylint max-module-lines setting
@vst vst merged commit 73d44d9 into main Jun 15, 2023
2 checks passed
@vst vst deleted the 18-improve-monetary-api branch June 15, 2023 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Monetary API
2 participants