-
Notifications
You must be signed in to change notification settings - Fork 10
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
Support dataclass default None as allowed type #99
Comments
Do dataclasses share the opinion that the |
So my understanding of the question is: are those two equivalent in deserialisation?
I guess it is a decision to make.
|
I see two questions. In no particular order... One question is 'should a default that doesn't fit the type hint be allowed'. I tend towards 'no' or maybe 'desert should not take a stand on this and leave it to underlying libraries'. I think that's the difference between not present (uses the default that disagrees with the type hint) and present but Another question is 'should I would think ending up with what the attrs issue discusses seems the cleanest. An optional attribute will get assigned a default of @dataclass
class WebhooksInfoItem:
url: typing.Optional[str] Do you see an issue or downside to writing your original attribute this way? (without making a claim this presently works, I haven't gotten into code at this point) |
I am still discovering the way to use the lib. I do think Optional is actually more suited to my purpose. (It also has the advantage that I do not need to bubble down all optional parameters to the bottom of my dataclass.) But I still think the error does not protect against much. Very interestingly, when using the python dataclass constructor itself the behavior is exactly reversed since it is from dataclasses import dataclass
from typing import Optional
import desert
@dataclass
class WebhooksInfoItem:
url: str = None
WebhooksInfoItemSchema = desert.schema(WebhooksInfoItem)
WebhooksInfoItemSchema.load({})
# Ok: WebhooksInfoItem(url=None) as there is a default value
WebhooksInfoItem()
# Ok: WebhooksInfoItem(url=None) as there is a default value
@dataclass
class WebhooksInfoItem:
url: Optional[str]
WebhooksInfoItemSchema = desert.schema(WebhooksInfoItem)
WebhooksInfoItemSchema.load({})
# Ok: WebhooksInfoItem(url=None) since it is Optional
# also WebhooksInfoItemSchema.dump(WebhooksInfoItemSchema.load({}))
# {"url":None}
WebhooksInfoItem()
# Error: TypeError: __init__() missing 1 required positional argument: 'url' This all might confuse a python beginner starting to use desert. |
Optional in a callable's signature means 'there is a default and you aren't required to pass this' while optional in typing means 'can also be None'. So they are two distinct things. So it sounds like you are satisfied with the functional path of The second example there relates to dataclasses implementing what was being discussed for attrs in python-attrs/attrs#460 I think? So that would not be a thing for desert to address. I would be interested to hear if you find any discussion of this. The other question raised here is what should be done with a default that disagrees with the type hint. So, should someone complain early when you write |
I agree with your points. |
To the strict interpretation of type specifications this class @dataclass
class MyData:
url: str = None is in conflict with its specification. The @dataclass
class MyData:
url: Optional[str]
MyData()
# TypeError: __init__() missing 1 required positional argument: 'url' is also expected. Giving it a type hint of url: Optional[str] = None |
Indeed in the end I would say I was confused about the fact |
The last confusing point comes from the fact that, as I stated, the dataclass way of handling things persists in an instance a field passed with None value in the same way that no field passed at all. This makes the schema dumps populating every field with Optional parameters (even if no None initialization defined) as such: @dataclass
class WebhooksInfoItem:
url: Optional[str]
WebhooksInfoItemSchema = desert.schema(WebhooksInfoItem)
WebhooksInfoItemSchema.dump(WebhooksInfoItemSchema.load({}))
# {"url":None} But I guess this is a python language decision (None cannot be distinguished from not passed) that this library can do little about. Still, is there a way in this library to inject marshmallow post_dump method to the schema instead as explained in https://github.com/marshmallow-code/marshmallow/issues/229#issuecomment-134387999? |
So, in my opinion, to go to the full extent of the interpretation that @altendky and @sveinse both stated, the dump should behave like this: @dataclass
class WebhooksInfoItem:
url: Optional[str]
id: Optional[str] = None
WebhooksInfoItemSchema = desert.schema(WebhooksInfoItem)
WebhooksInfoItemSchema.dump(WebhooksInfoItemSchema.load({}))
# {"id":None, "url": None} current behavior
# {"id":None} proposed behavior Following the schema definition rather than the dataclass instance value for the case of None at least. |
If you want to distinguish between I'm not clear what part of that example is what you feel we proposed vs. something you are proposing. Sparse serialization seems like a new topic to me. |
All I've got at the moment for a workaround to add post-dump is to generate the schema, add a post-dump to it by either inheritance or assignment, then manually specify that schema for any field where you want to use it. Sure, that's ugly. |
With this proposal, the behavior would change depending if the field happen to have a default value on an With the statement |
For what it's worth, my random two cents is that when we instantiate the My personal inuitition when I encountered this problem was that the |
I randomly landed on this thread not looking at the
The above proposed behavior keeps @dataclass
class WebhooksInfoItem:
url: Optional[str]
id: Optional[str] = None ...is equivalent to the below at runtime when you call and instantiate this class: class WebhooksInfoItem:
def __init__(self, url: Union[str,None], id: Union[str,None] = None):
self.url = url
self.id = id As you can see above, the Python at runtime sees @dataclass
class WebhooksInfoItem:
url: Optional[str]
id: Optional[str] = None
WebhooksInfoItem() - > WebhooksInfoItem(url=, id=None) #error
WebhooksInfoItem(None) -> WebhooksInfoItem(url=None, id=None) #OK
WebhooksInfoItem({}) -> WebhooksInfoItem(url={}, id=None) #OK
WebhooksInfoItem(id='123') -> WebhooksInfoItem(url=, id='123') #error
WebhooksInfoItem(id={}) -> WebhooksInfoItem(url=, id={}) #error A default value of any kind just gives you a nice shortcut so you don't have to pass a value yourself every time. It also allows you to define an appropriate default value (and I stress this next part) if one can be assumed. I stress this because usually any assumption for default value, even None vs not setting the attribute, is typically application specific, or potentially a business logic rather than a technical decision depending on the application purpose. You may want to look at the post_init method call for dataclasses instead as a possible alternative for what you're trying to accomplish and you can do type checking at runtime by utilizing I think these options would be a more Pythonic way for you to determine if the |
Hi, thanks for the nice library.
The following use case triggers an error in desert:
marshmallow_dataclass seems to support optional declaration through default None in both cases.
Is it forbidden in desert by design? It does not look like it according to the first usecase.
I feel that
= None
to be an implicit equivalent of Optional[] to be a good shortcut.Maybe I do not see a potential drawback?
The text was updated successfully, but these errors were encountered: