Skip to content

Conversation

daenny
Copy link
Contributor

@daenny daenny commented Sep 23, 2025

With this we can write better tests for the pixi-buid-ros backend.

Copy link
Contributor

@baszalmstra baszalmstra left a 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.

@daenny daenny force-pushed the feat/add-deserialize branch 2 times, most recently from 299e27f to ad8fa7e Compare September 23, 2025 08:46
@daenny
Copy link
Contributor Author

daenny commented Sep 23, 2025

You mean like this?
ad8fa7e

@daenny daenny changed the title feat(py-pixi): Add option to deserialize ProjectModelV1 from json feat(py-pixi): Add option to create ProjectModelV1 from json and dict Sep 23, 2025
@baszalmstra
Copy link
Contributor

Yeah!

Copy link
Contributor

@nichmor nichmor left a 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":
Copy link
Contributor

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

Copy link
Contributor Author

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.

@daenny daenny force-pushed the feat/add-deserialize branch 2 times, most recently from ddc1036 to 2e84d89 Compare September 23, 2025 14:47
@daenny daenny force-pushed the feat/add-deserialize branch from 2e84d89 to d1df14f Compare September 23, 2025 14:55
@tdejager
Copy link
Contributor

I see two approvals, so lets merge!

@tdejager tdejager merged commit 6e48656 into prefix-dev:main Sep 23, 2025
15 checks passed
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

Successfully merging this pull request may close these issues.

4 participants