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

Function to get minumum version of a dependency of a package #59

Closed
mauvilsa opened this issue Oct 14, 2022 · 3 comments · Fixed by #62
Closed

Function to get minumum version of a dependency of a package #59

mauvilsa opened this issue Oct 14, 2022 · 3 comments · Fixed by #62
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@mauvilsa
Copy link
Contributor

🚀 Feature

Add a function to lightning_utilities/core/imports.py that returns the minimum version of a dependency of a package.

Motivation

The minimum version of a dependency could be needed in more than one place in the code. When a change is needed, this needs to be done in multiple places. The issue is that it can be forgotten to update one location. An example is requirements/pytorch/extra.txt#L8 and src/pytorch_lightning/cli.py#L32.

Maybe this could be useful in other cases.

Pitch

Be able to change

_JSONARGPARSE_SIGNATURES_AVAILABLE = RequirementCache("jsonargparse[signatures]>=4.12.0")

to

jsonargparse_min_version = get_minimum_dependency_version("pytorch-lightning", "jsonargparse")
_JSONARGPARSE_SIGNATURES_AVAILABLE = RequirementCache(f"jsonargparse[signatures]>={jsonargparse_min_version}")

A possible implementation of this function could be:

def get_minimum_dependency_version(package_name: str, dependency_name: str) -> str:
    """Returns the minimum version of a dependency of a package.

    >>> get_minimum_dependency_version("pytorch-lightning", "jsonargparse") 
    '4.12.0'
    """
    for dependency in metadata.requires(package_name):
        if re.match(f"^{dependency_name}(|[^\w].*)$", dependency):
            return re.sub(r".*>=([\d.]+).*", r"\1", dependency)
    raise ValueError(f"dependency {dependency_name!r} not found in package {package_name!r}")

Alternatives

Keep updating multiple places in the code. When there is a mistake, fix it later.

Additional context

None

@mauvilsa mauvilsa added enhancement New feature or request help wanted Extra attention is needed labels Oct 14, 2022
@akihironitta
Copy link
Contributor

Hi @mauvilsa, thank you for your suggestion! It overall sounds good to me. One possible improvement is to update the error message to something like this so that users will know it's not their fault when they see the error:

-    raise ValueError(f"dependency {dependency_name!r} not found in package {package_name!r}")
+    raise ValueError(f"This is an internal error. Please file a GitHub issue with the error message. Dependency {dependency_name!r} not found in package {package_name!r}.")

Would you be interested in submitting a PR?

@carmocca
Copy link
Contributor

This could be included in the 0.4.0 release

@mauvilsa
Copy link
Contributor Author

Would you be interested in submitting a PR?

Yes, I can create a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants