Skip to content
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

feat: please consider not setting upper bounds on dependencies #10492

Open
1 task done
MarcoGorelli opened this issue Nov 14, 2024 · 5 comments · May be fixed by #10503
Open
1 task done

feat: please consider not setting upper bounds on dependencies #10492

MarcoGorelli opened this issue Nov 14, 2024 · 5 comments · May be fixed by #10503
Labels
feature Features or general enhancements

Comments

@MarcoGorelli
Copy link

MarcoGorelli commented Nov 14, 2024

Is your feature request related to a problem?

Ibis currently sets upper bounds on dependencies. E.g. the latest release pins pyarrow<18

What is the motivation behind your request?

This causes issues in CI. For example here it was blocking upgrading to Python 3.13

Describe the solution you'd like

I'd suggest not setting upper bounds on dependencies, as suggested here https://iscinumpy.dev/post/bound-version-constraints/

Capping dependencies has long term negative effects, especially for libraries, and should never be taken lightly. A library is not installed in isolation; it has to live with other libraries in a shared environment. Only add a cap if a dependency is known to be incompatible or there is a high (>75%) chance of it being incompatible in its next release. Do not cap by default - capping dependencies makes your software incompatible with other libraries that also have strict lower limits on dependencies, and limits future fixes.

What version of ibis are you running?

9.5.0

What backend(s) are you using, if any?

Polars

Code of Conduct

  • I agree to follow this project's Code of Conduct
@cpcloud
Copy link
Member

cpcloud commented Nov 14, 2024

We can probably do this for pyarrow. IIRC pyarrow releases don't often break Ibis, we're not using that much of its API, and the parts we are using have been around for years.

Doing this for every dependency is not something we should do, given that some of them regularly break Ibis (e.g., sqlglot) and require us to write compatibility code to work around the breakage.

@acuitymd-filip
Copy link

That would be amazing to at least lift this for pyarrow - we want to update to pyarrow 18 to benefit from the new string_view stuff in that release, but currently blocked by ibis 9.5 requiring pyarrow<18...

@deepyaman deepyaman linked a pull request Nov 16, 2024 that will close this issue
@NickCrews
Copy link
Contributor

I just wanted to include this sqlglot PR, so I tried to add sqlglot>=25.30.0 as a dependency to my app. But my app also depends on the latest stable release of ibis, which is 9.5.0. But ibis 9.5.0 has a range of >=23.4,<25.21, implemented by dependabot in #10050. This incompatibility prevents me from uv locking my app.

I hear you on keeping upper bounds when we KNOW that there is an incompatibility. But it looks to me like dependabot was bumping the upper bound to the latest released version. ie it was speculating that all future releases would be incompatible. I suppose that running sqlglot==25.30.0 at the same time as ibis-framework==9.5.0 may be buggy, and ME as the end user wouldn't notice, because I don't have a comprehensive enough test suite, and thus I get incorrect behavior. Only if I ran the entire ibis test suite would I notice the problem. So I guess you could be protecting me from shooting myself in the foot, but I would think it is much more likely that an incompatibility would cause a crash rather than incorrect behavior, and that the benefits of pulling in a needed patch would outweigh the risks.

Is there some other workaround that you know of where I can use sqlglot>=25.30.0 at the same time as ibis-framework==9.5.0? I could also wait for the weekly ibis pre-release to drop on pypi.

@MarcoGorelli
Copy link
Author

I hear you on keeping upper bounds when we KNOW that there is an incompatibility

Yup, same. To quote the article above

Do not cap by default - capping dependencies makes your software incompatible with other libraries that also have strict lower limits on dependencies, and limits future fixes. Anyone can fix a missing cap, but users cannot fix an over restrictive cap causing solver errors.

I'd appreciate it if could also not cap:

  • atpublic
  • parsy
  • python-dateutil
  • toolz
  • typing-extensions

thanks 🙏

@deepyaman
Copy link
Contributor

deepyaman commented Nov 29, 2024

PyArrow upper bound is also blocking 3.13 support in Kedro-Datasets.

Cc @noklam xref #10381 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Features or general enhancements
Projects
Status: backlog
Development

Successfully merging a pull request may close this issue.

5 participants