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

Getting currency instance from its code #17

Closed
flaeppe opened this issue Sep 15, 2022 · 6 comments · Fixed by #18
Closed

Getting currency instance from its code #17

flaeppe opened this issue Sep 15, 2022 · 6 comments · Fixed by #18

Comments

@flaeppe
Copy link
Collaborator

flaeppe commented Sep 15, 2022

If I'm not missing something, I don't think I can get a Currency instance from its code? Is that something that could be natively supported?

Reasoning is that I'm looking in to support for a Currency type for a pydantic model field. So I also have a follow up question on if supporting pydantic would be of interest?

I could probably monkey patch the Currency type to support pydantic while also mapping up code -> Currency to get it working. But was curious if any of the above support would be interesting to package.

@antonagestam
Copy link
Owner

@flaeppe You're correct, that's not supported yet, and I agree it should be!

I did start tinkering with it a little bit on this branch: https://github.com/antonagestam/immoney/tree/feature/currency-registry

I wanted to implement it in a somewhat generic way, so that it's not mandatory to import and use immoney.currencies, but you could create your own module with your own set of currency definitions (that's why there's no magic self-registration on subclassing Currency in that branch).

Implementing Pydantic parsing should be straight-forward, but serialization probably needs a hack in user code to map Money to a json serializer. For that reason I'm thinking support for Pydantic 1.x should be provisional and be removed once Pydantic 2.x is out and brings better support for custom type adapters (see pydantic/pydantic#951 (comment), thread and linked document).

For now, if you'd like to work on any of the above, feel free to go ahead! But as stated, support for Pydantic 1.x might be ruthlessly removed once 2.x is out (I'm ok with adding it as a dependency if needed for now though).

Also note that I think the serialized format for Money objects should be a nested object and not represented as two separate fields on the model object. E.g.:

class Foo(BaseModel):
    foo: str
    value: Money

Becomes:

{
    "foo": "foo",
    "value": {"value": "23.40", "currency": "SEK"},
}

@flaeppe
Copy link
Collaborator Author

flaeppe commented Sep 15, 2022

For the requirements I have, right now, only support for Currency is needed. I'm guessing I'd start in that end, if so.

I suppose, for the time being, one could also take a little bit more manual/verbose approach of doing something like below, for Money

class Foo(BaseModel):
    amount: Decimal
    currency: Currency

    @root_validator()
    def validate_is_money(cls, values: dict[str, Any]) -> dicr[str, Any]:
        try
            Money(values.get("amount"), values.get("currency"))
        except SomeException as exc:
            raise ValidationError() from exc
        return values

    @cached_property
    def money(self) -> Money:
        return Money(self.amount, self.currency)

(I suppose there could exist some corresponding variant using private model attributes as well)

But I get the reasoning of being able to serialize/parse as a Money type directly.

Anyways, some currency registry and currency supported as pydantic type would be great.

@antonagestam
Copy link
Owner

@flaeppe I'll probably be able to get a basic currency registry out soon :)

@antonagestam
Copy link
Owner

@flaeppe Does this look like it will get you at least half of the way there? 🙂

#18

@flaeppe
Copy link
Collaborator Author

flaeppe commented Sep 15, 2022

Looks great!

@antonagestam
Copy link
Owner

@flaeppe This is now being published in 0.1.0. I created a separate issue to track progress for Pydantic support: #20. Thanks for the help! 🙏

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 a pull request may close this issue.

2 participants