cornellius-gp / linear_operator

A LinearOperator implementation to wrap the numerical nuts and bolts of GPyTorch
MIT License
95 stars 28 forks source link

Typeguard 4.3.0 #97

Closed ChristianWirthContinental closed 3 months ago

ChristianWirthContinental commented 4 months ago

Fixes #84 by updating typeguard to 4.X

This is important because the typeguard requirement is in conflict with ydata-profiling: https://github.com/ydataai/ydata-profiling/blob/cc7b9b36364fdcb6db6a23db5d83ad3c6e2e3c8f/requirements.txt#L23 - a commonly used EDA package. This, in turn, prevents updates to pandas and numpy.

All unittests passed

tmct commented 4 months ago

Hang on a minute, this is only used in tests? We can move this from install_requires to being a dependency of only the test environment!

And - it looks like >=3.1,<5 might be a nice balance, given that the commented out code here wants to use that minimum

tmct commented 4 months ago

Probably easier to describe that by making the change - what do you think? #98

(And if it's a test requirement, we don't have to over think the permissiveness of the lower bound!)

ChristianWirthContinental commented 4 months ago

Probably easier to describe that by making the change - what do you think? #98

(And if it's a test requirement, we don't have to over think the permissiveness of the lower bound!)

I think your solution is good, as this fully resolves the conflict also for the future (as long as one is not doing testing). However, i'm fine with both PRs and just want to see the problem resolved 😄

ChristianWirthContinental commented 3 months ago

Moving forward with this or or #98 would be of great help, as it blocks some sever downstream issues. Anything preventing this from the next steps? (@gpleiss ?)

tmct commented 3 months ago

We are also eager to see either change made, please. (In the meantime we have been using uv's dependency overrides to create functioning environments.)

gpleiss commented 3 months ago

@ChristianWirthContinental @tmct the locked version of typeguard was intentional (but poorly documented in the code). Newer versions of typeguard aren't compatible with jaxtyping. See:

It's worth noting that typeguard versioning has caused previous issues. As a workaround, we can require the specific locked version of typeguard in our testing code (where we need the specific version for jaxtyped tests) but we don't have to lock down typeguard for the main package dependencies (where jaxtyping is strictly used for annotations only).

tmct commented 3 months ago

Thank you!

n.b. if you ever upgrade your min version of jaxtyping beyond I think 0.2.22, as things stand I think we'd hit the same problem again through transitive dependencies being tightened... So hopefully jaxtyping can be made compatible before then!

ChristianWirthContinental commented 3 months ago

@gpleiss Thank you very much! If you don't mind releasing this, i would try to get the new version into botorch to resolve the version conflicts in the dependency chain.

gpleiss commented 3 months ago

Done