fix: no blank children names #666
Conversation
wrote this when attempting to debug this issue
while trying to understand the codebase
This reverts commit 075a4ed. Causing issues with old py
| field_name="", | ||
| # this is a fake field name, but it makes it possible to debug which type variant | ||
| # is the source of an exception downstream | ||
| field_name=f"{field_name}__{arg.__name__}", |
There was a problem hiding this comment.
is there a reason we couldn't
- just use
field_name - specify
arg.__name__as a separate field on field_meta
current approach here feels bad
There was a problem hiding this comment.
Currently, I believe the main use is mapping this back to the model itself. For subfields, guessing currently unused so field_name could work and feels preferable to above.
What is the issue with having no field name? Is this to improve exception message based on test? If that is the case then maybe could update on that side instead
There was a problem hiding this comment.
What is the issue with having no field name?
I ran into this when attempting to run a FactoryModel.build() command. The ParameterException said that a factory for a type was missing, but it the name was blank, so I couldn't determine which field was causing the error.
For subfields, guessing currently unused so
field_namecould work and feels preferable to above
Great, I'll change the PR to just use field_name instead.
There was a problem hiding this comment.
I ran into this when attempting to run a FactoryModel.build() command. The ParameterException said that a factory for a type was missing, but it the name was blank, so I couldn't determine which field was causing the error.
Where is that error coming from? Does the type matter there?
Probably clearer to have field_name regardless
| @@ -106,7 +106,7 @@ def is_new_type(annotation: Any) -> "TypeGuard[type[NewType]]": | |||
|
|
|||
|
|
|||
| def is_annotated(annotation: Any) -> bool: | |||
| """Determine whether a given annotation is 'typing.Annotated'. | |||
| """Determine whether a given annotation is 'typing.Annotated', or a similar typing annotation such as Optional. | |||
There was a problem hiding this comment.
Is this correct? The function looks for annotated origin or similar
There was a problem hiding this comment.
this is why i left this comment, it's pretty tricky
In [12]: a
Out[12]: typing.Optional[int]
In [13]: a.__args__
Out[13]: (int, NoneType)
In [14]: b = int | None
Out[14]: int | None
In [15]: b.__args__
Out[15]: (int, NoneType)
There was a problem hiding this comment.
Does this match _AnnotatedAlias or other conditions?
There was a problem hiding this comment.
I'm not familiar with _AnnotatedAlias, not sure...
There was a problem hiding this comment.
Based on testing in terminal think this handles this correctly so fine as is?
>>> from polyfactory.utils.predicates import is_annotated
>>> from typing import Optional
>>> is_annotated(Optional[int])
False
>>> is_annotated(int | None)
False
| class OptionalFormTwo(BaseModel): | ||
| optional_custom_type_second_form: CustomType | None | ||
|
|
||
| @classmethod |
There was a problem hiding this comment.
Is this needed? This is not used by the factory
Description
ensure no blank field names on children variants