search

apache / iceberg-python

Apache PyIceberg
https://py.iceberg.apache.org/
Apache License 2.0
309 stars 114 forks source link

Fix deepcopy of primitive types #857

Closed ndrluis closed 5 days ago

ndrluis commented 1 week ago

The IcebergRootModel inherits from Pydantic RootModel, which has its own implementation of deepcopy. When deepcopy runs, it calls this __deepcopy__ method and ignores that it's a Singleton. So, my solution was to change the order of inheritance and implement a __deepcopy__ method for singletons that returns itself.

https://github.com/pydantic/pydantic/blob/f024d03b832d1bbcbadf76184ed14d92571a71ca/pydantic/root_model.py#L108-L116

ndrluis commented 1 week ago

Thank you, @syun64. I have added the test and used an existing metadata definition to avoid duplication.

Regarding the solution, to be honest, it was not quick. It took me several weeks of learning and then stepping away from the problem for a few weeks to return with fresh ideas on how to solve it.

syun64 commented 1 week ago

Hi @HonahX @Fokko could we ask for your help in triggering this workflow?

ndrluis commented 6 days ago

@HonahX @Fokko, can you please trigger the workflow? I have already run the tests in my fork and everything is green.

image
Fokko commented 6 days ago

@ndrluis Thanks for pinging me here. This approval should be gone once the first PR has been merged. Which should be soon 🚀

Fokko commented 5 days ago

@HonahX Sorry for the ambigous comment there. I was referring to approving the CI to run, which is very annoying. Now @ndrluis has his first PR in, this approval should be gone and the CI should be kicked off right away.

Fokko commented 5 days ago

Thanks @ndrluis for working on this, and @syun64, @kevinjqliu and @HonahX for the quick review 🚀