feat(bigquery-magics): drop support for Python 3.9#16587
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the package dependencies and drops support for Python 3.9, raising the minimum requirement to Python 3.10. Review feedback highlights that bigframes and geopandas were inadvertently removed from the test sessions and constraints, and questions the addition of pyopenssl as a mandatory dependency. Additionally, it is suggested to remove Python 3.9 entirely from the configuration lists rather than keeping it for explicit skipping, and to ensure that breaking changes regarding environment support are accompanied by appropriate versioning.
| "3.10": [ | ||
| "bqstorage", | ||
| "bigframes", | ||
| "geopandas", | ||
| ], |
There was a problem hiding this comment.
The dependencies bigframes and geopandas were removed from the Python 3.10 test session. Since 3.10 is now the minimum supported version, these extras should continue to be tested to ensure compatibility. If they were moved elsewhere, please clarify; otherwise, they should be restored to maintain test coverage.
"3.10": [
"bqstorage",
"bigframes",
"geopandas",
],There was a problem hiding this comment.
This is intentional. We need a minimimal set of extras so that the minimum versions of dependencies can be properly tested.
| "3.10", | ||
| "3.11", | ||
| "3.12", | ||
| "3.13", | ||
| "3.14", | ||
| # Not supported, but included so that we can explicitly skip the session | ||
| # from here. Keep unsupported versions last so that they don't conflict with | ||
| # the prerelease_deps session. | ||
| "3.9", | ||
| ] |
There was a problem hiding this comment.
Instead of keeping Python 3.9 in the list and manually skipping it in every session, it is cleaner to remove it from UNIT_TEST_PYTHON_VERSIONS entirely. Note that dropping support for Python 3.9 is a breaking change; per repository rules, this should be handled with a minor version bump rather than a patch to allow for continued support of the previous environment. Additionally, Python 3.14 should be included in the versions list as it is a valid version for this repository.
"3.10",
"3.11",
"3.12",
"3.13",
"3.14",
]References
- When a release introduces breaking changes in environment requirements, such as dropping support for specific Python versions, prefer a minor version bump over a patch version bump.
- Python 3.14 is a valid Python version and should be supported.
There was a problem hiding this comment.
Can't do until the global github actions config is updated.
| if session.python == "3.9": | ||
| session.skip("Python 3.9 is not supported.") |
There was a problem hiding this comment.
This skip block is unnecessary if Python 3.9 is removed from the UNIT_TEST_PYTHON_VERSIONS list.
| if session.python == "3.9": | ||
| session.skip("Python 3.9 is not supported.") |
There was a problem hiding this comment.
This skip block is unnecessary if Python 3.9 is removed from the UNIT_TEST_PYTHON_VERSIONS list.
| "pyarrow >= 12.0.0", | ||
| "pydata-google-auth >=1.5.0", | ||
| "tqdm >= 4.7.4, <5.0.0", | ||
| "pyopenssl >= 23.3.0", |
There was a problem hiding this comment.
Adding pyopenssl as a mandatory dependency is a significant change, as it requires C extensions and is generally not needed for Python 3.10+. Could you explain why this is now required for all users? If it's only needed for specific edge cases, consider making it an optional extra.
There was a problem hiding this comment.
Several unit tests fail without this change. Note that it is a required dependency in later versions of google-auth, anyway.
| # This constraints file is used to check that lower bounds | ||
| # are correct in setup.py | ||
| # List *all* library dependencies and extras in this file. | ||
| # Pin the version to the lower bound. | ||
| # | ||
| # e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev", | ||
| # Then this file should have foo==1.14.0 | ||
| db-dtypes==1.1.1 | ||
| geopandas==1.0.1 | ||
| google-cloud-bigquery==3.13.0 | ||
| google-cloud-bigquery-storage==2.6.0 | ||
| ipywidgets==7.7.1 | ||
| ipython==7.23.1 | ||
| ipykernel==5.5.6 | ||
| numpy==1.26.4 | ||
| packaging==20.0.0 | ||
| pandas==1.5.3 | ||
| pyarrow==12.0.0 | ||
| pydata-google-auth==1.5.0 | ||
| pyopenssl==23.3.0 | ||
| tqdm==4.7.4 |
There was a problem hiding this comment.
bigframes was removed from the constraints file. If it remains a supported extra for this package, it should be included here at its minimum supported version to ensure that lower-bound compatibility is verified in CI.
There was a problem hiding this comment.
bigframes requires a newer version of the client library than I would like to upgrade to at this time.
Related to b/490130968 and b/470438395
🦕