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

typing.Optional seems to be required for optional fields (while README implies otherwise) #32

Open
githuib opened this issue Feb 16, 2024 · 1 comment

Comments

@githuib
Copy link

githuib commented Feb 16, 2024

I actually managed to work around this issue, but just to notify you there seems to be some incorrect parts in the README docs. And to share some suggestions. I could make a pull request for both but I'm not sure what your thoughts are about it.

Edit: I just noticed it's possible to do what I want with dataclasses and chili.decoder import ClassDecoder (or just chili.decode() directly), as is shown in https://github.com/kodemore/chili/blob/main/tests/usecases/dataclasses_test.py#L140-L147
Which is perfect for my use case.

Below my original message, which could still be interesting:

When I try to decode a dict to a class while omitting a field that does have a default defined, I get a chili.error.DecoderError@missing_property

For example when I run the example straight from the README:

from typing import List
from chili import Decoder, decodable

@decodable
class Book:
    name: str
    author: str
    isbn: str = "1234567890"
    tags: List[str] = []

book_data = {"name": "The Hobbit", "author": "J.R.R. Tolkien"}
decoder = Decoder[Book]()

book = decoder.decode(book_data)

I get: chili.error.DecoderError@missing_property: key=isbn

I noticed in this unit test the optional fields are typed with Optional[SomeType], which indeed fixes the issue.
Although this seems to make sense at first given the name "Optional", it does make it possible to set the field to None, which might not always be as intended. A custom decoder could be used to parse 'None' to the default value, although it would be a bit more convenient if this would be done automatically by specifying a default while leaving out Optional.

Besides, it would be nice to be able to use the more recent SomeType | None syntax as an alternative to Optional[SomeType].

I also noticed that @encodable & @decodable seem to be redundant (or at least in the cases from the examples), making it possible to keep most parts of the code completely unaware of the chili lib, which I like a lot 👍

@dkraczkowski
Copy link
Contributor

@githuib Hey, thanks for your feedback. I need to review the documentation soon. Changes have been made to simplify the interface, which may cause discrepancies in the documentation. My current focus is on implementing features that are directly beneficial for our daily work, due to limited time availability.

I welcome contributions to update or correct the documentation if you notice any discrepancies and have the time to address them.

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

No branches or pull requests

2 participants