search

JakobGM / patito

A data modelling layer built on top of polars and pydantic
MIT License
270 stars 23 forks source link

fix: update LDF.collect() for polars==0.19.8 #24

Closed brendancooley closed 5 months ago

brendancooley commented 11 months ago

polars removed *kwargs from LazyFrame.collect() in the 0.19.8 release. Since the signature for patito's child LDF.collect does not match that of pl.LazyFrame.collect, the test_dummy_data suite fails after upgrading polars. This PR moves collect to an `args, **kwargspattern for better backward/forward compatibility. Tests pass withpolars==0.19.8andpolars==0.18.7`.

ion-elgreco commented 10 months ago

Thanks I also ran into the issue, I've merged this fix into my own develop branch of patito fork. There isn't much activity in this repo anymore it seems.

thomasaarholt commented 10 months ago

We're working on a migration to pydantic v2, it's just difficult to squeeze in development time between our jobs.

ion-elgreco commented 10 months ago

We're working on a migration to pydantic v2, it's just difficult to squeeze in development time between our jobs.

Wouldn't it then make sense to at least merge all the fixes, so the current release works as expected?

thomasaarholt commented 10 months ago

We have a work-in-progress branch that we are migrating the pydantic work with, and it is extensive enough that we didn't want to risk having to do rebases while still doing that. I agree that the fixes here are valuable, but we decided to prioritize the pydantic migration first.

ion-elgreco commented 10 months ago

We have a work-in-progress branch that we are migrating the pydantic work with, and it is extensive enough that we didn't want to risk having to do rebases while still doing that. I agree that the fixes here are valuable, but we decided to prioritize the pydantic migration first.

Are all the current issues accounted for in the Pydantic v2 release or will the same issues persist?

thomasaarholt commented 10 months ago

We will likely have fixed some issues, while others will persist. We will rebase the PRs on the pydantic-migrated-version and then go through the PRs one by one.

ion-elgreco commented 10 months ago

We will likely have fixed some issues, while others will persist. We will rebase the PRs on the pydantic-migrated-version and then go through the PRs one by one.

If you need any help to port the fixes to the pydantic-migrated version, I am happy to help there. We use Patito quite extensively in our codebase

thomasaarholt commented 5 months ago

This is now fixed in the main branch :)