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

Lack of requires-python causes tfp v0.20.0 to break all Python 3.7 and older #1723

Closed
matthewfeickert opened this issue May 16, 2023 · 8 comments · Fixed by #1724
Closed

Comments

@matthewfeickert
Copy link
Contributor

matthewfeickert commented May 16, 2023

Hi. As TensorFlow Probability doesn't have requires-python metadata (c.f. https://peps.python.org/pep-0621/#requires-python) the recent v0.20.0 release has broken installation for Python 3.7 and older for everyone (c.f. e81e7e8). This is because TensorFlow v2.12.0 is the first TensorFlow release to require Python 3.8+. So requiring

required_tensorflow_version = '2.12'

without properly communicating that Python 3.8+ is now explicitly required by TensorFlow probability means that any Python 3.7 (or older) that was able to install and use TensorFlow v2.11.0 and previous versions of TensorFlow Probability now gets a broken install of tensorflow v2.11.0 and tensorflow-probability v0.20.0 which produce a runtime ImportError

This version of TensorFlow Probability requires TensorFlow version >= 2.12; Detected an installation of version 2.11.0. Please upgrade TensorFlow to proceed.

(Example: scikit-hep/pyhf#2202)

Including requires-python metadata would have properly guarded against this behavior. TensorFlow Probability v0.20.0 should be yanked from PyPI and a v0.20.1 should be cut that includes PR #1725 (backport of PR #1724).

These patterns have been discussed in many places previously and in great detail in @henryiii's blog post "Should You Use Upper Bound Version Constraints?" (c.f. https://iscinumpy.dev/post/bound-version-constraints/#pinning-the-python-version-is-special for somewhat similar ideas).

cc @jburnim who made the release.

@matthewfeickert
Copy link
Contributor Author

matthewfeickert commented May 16, 2023

Thanks for the fast merge of PR #1724. 👍 Though for this Issue to be fully closed out the following still needs to happen:

@ColCarroll
Copy link
Contributor

Thanks for reporting and fixing @matthewfeickert -- super helpful!

We're working on the above -- waiting for timezones to make it to the proper coasts for doing all the things 😁

I'll reopen this to keep track in the meantime.

@ColCarroll ColCarroll reopened this May 16, 2023
@matthewfeickert
Copy link
Contributor Author

matthewfeickert commented May 16, 2023

We're working on the above -- waiting for timezones to make it to the proper coasts for doing all the things 😁

I'll reopen this to keep track in the meantime.

Thanks @ColCarroll! :) I also 100% understand working across timezones. ;) You're all moving quite fast on this in my mind (so thank you!) and any additional notes are more for my own benefit so that I get them out while they're still in-flight in my mind. 👍

@matthewfeickert
Copy link
Contributor Author

@jburnim thanks for releasing tensorflow-probability v0.20.1 quickly. 👍 It is still important that v0.20.0 is yanked from PyPI as without that all installs on Python 3.7 and older will continue to be broken.

@henryiii
Copy link

FYI, if it helps, yanking is a safe procedure; anyone who has pinned v0.20.0 will still be able to get it; it will only no longer participate in open solves (which is what you want here).

@matthewfeickert
Copy link
Contributor Author

FYI, if it helps, yanking is a safe procedure;

Agreed, though I think the team already knows this as they have previously yanked https://pypi.org/project/tensorflow-probability/0.16.0.dev20220214/

@jburnim
Copy link
Member

jburnim commented May 17, 2023

The 0.20.0 release has been yanked.

@jburnim jburnim closed this as completed May 17, 2023
@matthewfeickert
Copy link
Contributor Author

Excellent. Thank you @jburnim!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants