-
Notifications
You must be signed in to change notification settings - Fork 19
feat(py-pixi): Add option to create ProjectModelV1
from json and dict
#373
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldnt it be more pythonic and easier to be able to deserialize from a python dictionaries instead of a json string? You can use the phytonize crate for that.
299e27f
to
ad8fa7e
Compare
You mean like this? |
ProjectModelV1
from jsonProjectModelV1
from json and dict
Yeah! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one small comment - apart from that thanks a lot!
return instance | ||
|
||
@classmethod | ||
def from_dict(cls, data: Mapping[str, Any] | dict[str, Any]) -> "ProjectModelV1": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be Dict
instead of dict
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I adjusted it for now for consistency.
I think, if as we discussed on discord target minimum python 3.10, then all the typing should go to the builtins:
https://peps.python.org/pep-0585/#id6
The deprecated functionality may eventually be removed from the typing module. Removal will occur no sooner than Python 3.9’s end of life, scheduled for October 2025.
ddc1036
to
2e84d89
Compare
2e84d89
to
d1df14f
Compare
I see two approvals, so lets merge! |
With this we can write better tests for the
pixi-buid-ros
backend.