MISC: Check that min versions are aligned in CI and import_optional_dependency #45219
Conversation
|
when you update pre-commit hooks, does one have to update dev environment with some git command, or is the addition of new files and new code enough for the hooks to operate as expected? |
Nope. pre-commit will run (and install new dependencies if needed) with the changes of |
There was a problem hiding this comment.
LGTM. Someone more familiar with the pre-commit hooks might want to review this though.
cc @MarcoGorelli.
| @@ -59,6 +66,9 @@ def get_version(module: types.ModuleType) -> str: | |||
|
|
|||
| if version is None: | |||
| raise ImportError(f"Can't determine version for {module.__name__}") | |||
| if module.__name__ == "psycopg2": | |||
There was a problem hiding this comment.
can you integrate this to L64, e.g. maybe make a separate make of module -> lambda functions that can handle the special case modules?
There was a problem hiding this comment.
Hmm the L64 block handles if __version__ is not the correct path to the version. psycopg2 uses __version__ so it wouldn't hit this path
| import version | ||
|
|
||
| sys.modules["pandas.util.version"] = version | ||
| import _optional |
There was a problem hiding this comment.
can you import these as fully qualified and/or make this whole section a bit more idiomatic (i know tricky, but give a try)
There was a problem hiding this comment.
Tried playing around with this but couldn't simplify it further than what @lithomas1 suggested.
|
also pls rebase |
|
@mroeschke can you rebase this to make sure ok (as merged @lithomas1 change) |
Yes it appears so. |
|
it's not urgent (i.e. block 1.4) but would be good to keep the ci files in sync on 1.4.x and main until they need to diverge for say Python 3.11 so that ci backports have less chance of needing manual intervention. could either sync ci/deps/actions-38-minimum_versions.yaml or backport this PR. |
|
Might be better to backport this PR since it has some additional changes to |
|
@meeseeksdev backport 1.4.x |
…ed in CI and import_optional_dependency
…nd import_optional_dependency (#45537) Co-authored-by: Matthew Roeschke <emailformattr@gmail.com>
Follow up to include
install.rst