Skip to content
This repository has been archived by the owner on Mar 16, 2024. It is now read-only.

Model fix #56

Merged
merged 10 commits into from
Jul 3, 2022
Merged

Model fix #56

merged 10 commits into from
Jul 3, 2022

Conversation

twistfire92
Copy link
Contributor

add default value for extra_fields

@hall-b
Copy link

hall-b commented Jun 21, 2022

Bad idea to initialize directly with a dictionary.
Cf: https://towardsdatascience.com/python-pitfall-mutable-default-arguments-9385e8265422
Better do: extra_fields: Optional[dict] = Field(default_factory=lambda: dict())

@yannicschroeer
Copy link
Collaborator

yannicschroeer commented Jun 21, 2022

Agreed, default values for dictionaries should either be a factory method or None (Which it was before) due to their mutability. Anyway, why is a default value necessary?

@hall-b
Copy link

hall-b commented Jun 21, 2022

I guess if the value is not initialized and thus equal to None, it will raise an error here when trying to set a value

@twistfire92
Copy link
Contributor Author

Bad idea to initialize directly with a dictionary.
Cf: https://towardsdatascience.com/python-pitfall-mutable-default-arguments-9385e8265422
Better do: extra_fields: Optional[dict] = Field(default_factory=lambda: dict())

Or just Field(default_factory=dict)

@yannicschroeer
Copy link
Collaborator

Bad idea to initialize directly with a dictionary.
Cf: https://towardsdatascience.com/python-pitfall-mutable-default-arguments-9385e8265422
Better do: extra_fields: Optional[dict] = Field(default_factory=lambda: dict())

Or just Field(default_factory=dict)

I'd prefer this. No need for a lambda

@twistfire92
Copy link
Contributor Author

Bad idea to initialize directly with a dictionary.
Cf: https://towardsdatascience.com/python-pitfall-mutable-default-arguments-9385e8265422
Better do: extra_fields: Optional[dict] = Field(default_factory=lambda: dict())

Or just Field(default_factory=dict)

I'd prefer this. No need for a lambda

fixed!

@yannicschroeer yannicschroeer merged commit 3d9b35e into code-specialist:master Jul 3, 2022
@twistfire92 twistfire92 deleted the model_fix branch August 23, 2022 06:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants