search

dagster-io / dagster

An orchestration platform for the development, production, and observation of data assets.
https://dagster.io
Apache License 2.0
10.73k stars 1.33k forks source link

dagster-duckdb-polars lists polars as a dependency #19736

Open EtienneT opened 4 months ago

EtienneT commented 4 months ago

Dagster version

1.6.4

What's the issue?

The polars project has two released package, polars and polars-lts-cpu. You have to use polars-lts-cpu when using an older CPU like the Intel Xeon processors. Both package are maintained by the official polars team.

dagster-duckdb-polars lists polars directly as a dependency which force polars to get installed when we install all dependencies. This works fine in development where my desktop computer has a fairly recent processor. But when deploying in production where the server has a Xeon processor, both package conflict since they are both imported the same way, import polars as pl.

What did you expect to happen?

Ideally we would remove polars from the dependencies of dagster-duckdb-polars and let the user choose which package to install, either polars or polars-lts-cpu.

How to reproduce?

No response

Deployment type

Local

Deployment details

No response

Additional information

No response

Message from the maintainers

Impacted by this issue? Give it a 👍! We factor engagement into prioritization.

maarek commented 1 month ago

Is this something that can be tackled? It causes issues with development on Apple M1 arch when using polars under rosetta and integrating with dagster-duckdb-polars packages.

danielgafni commented 1 month ago

Seems like both dependencies should be added but constrained by environment markers (platform_machine).

This way only one of them will get installed depending on current CPU architecture.

Something like (the exact logic will be more complex to work for different architectures):

setup(
  install_requires=[
    "polars : platform.machine() == 'x86_64'", 
    "polars-lts-cpu : platform.machine() != 'x86_64'"
  ],
)
EtienneT commented 1 month ago

Linking relevant issue in polars. There doesn't seem to be a concensus on the solution yet: https://github.com/pola-rs/polars/issues/12880