-
Notifications
You must be signed in to change notification settings - Fork 874
fix(tests): Minor fixes #3327
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
fix(tests): Minor fixes #3327
Changes from 7 commits
4bd1b62
703e864
01da6ae
991f65f
de0e040
d398988
bbf5046
7495201
17fcaf3
76743ff
d5efe21
1830ced
eb5fdae
1b7d4bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -178,5 +178,5 @@ lightning_logs/ | |
| mlruns | ||
|
|
||
| # application data | ||
| data/ | ||
| logs/ | ||
| application/data/ | ||
| application/logs/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,8 @@ | |
| from pathlib import Path | ||
| from warnings import warn | ||
|
|
||
| from pkg_resources import Requirement | ||
| from packaging.requirements import Requirement | ||
| from packaging.version import Version | ||
|
|
||
| AVAILABLE_TORCH_VERSIONS = { | ||
| "2.0.0": {"torchvision": "0.15.1", "cuda": ("11.7", "11.8")}, | ||
|
|
@@ -66,7 +67,7 @@ def get_requirements(module: str = "anomalib") -> dict[str, list[Requirement]]: | |
| if isinstance(requirement_extra, list) and len(requirement_extra) > 1: | ||
| extra = requirement_extra[-1].split("==")[-1].strip("'\"") | ||
| requirement_name_ = requirement_extra[0] | ||
| requirement_ = Requirement.parse(requirement_name_) | ||
| requirement_ = Requirement(requirement_name_) | ||
| if extra in extra_requirement: | ||
| extra_requirement[extra].append(requirement_) | ||
| else: | ||
|
|
@@ -114,9 +115,9 @@ def parse_requirements( | |
| other_requirements: list[str] = [] | ||
|
|
||
| for requirement in requirements: | ||
| if requirement.unsafe_name == "torch": | ||
| if requirement.name.lower() == "torch": | ||
| torch_requirement = str(requirement) | ||
| if len(requirement.specs) > 1: | ||
| if len(requirement.specifier) > 1: | ||
| warn( | ||
| "requirements.txt contains. Please remove other versions of torch from requirements.", | ||
ashwinvaidya17 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| stacklevel=2, | ||
|
|
@@ -333,20 +334,27 @@ def get_torch_install_args(requirement: str | Requirement) -> list[str]: | |
| True | ||
| """ | ||
| if isinstance(requirement, str): | ||
| requirement = Requirement.parse(requirement) | ||
| requirement = Requirement(requirement) | ||
|
|
||
| # NOTE: This does not take into account if the requirement has multiple versions | ||
| # such as torch<2.0.1,>=1.13.0 | ||
| if len(requirement.specs) < 1: | ||
| if not requirement.specifier: | ||
| return [str(requirement)] | ||
| select_spec_idx = 0 | ||
| for i, spec in enumerate(requirement.specs): | ||
| if "=" in spec[0]: | ||
| select_spec_idx = i | ||
| preferred_operators = ("==", "<=", "<", ">=", ">") | ||
| selected_spec = None | ||
| for operator in preferred_operators: | ||
| for spec in requirement.specifier: | ||
| if spec.operator == operator: | ||
| selected_spec = spec | ||
| break | ||
| if selected_spec is not None: | ||
| break | ||
| operator, version = requirement.specs[select_spec_idx] | ||
| if selected_spec is None: | ||
| selected_spec = next(iter(requirement.specifier)) | ||
|
||
| operator = selected_spec.operator | ||
| version = selected_spec.version | ||
|
Comment on lines
342
to
+359
|
||
| if version not in AVAILABLE_TORCH_VERSIONS: | ||
| version = max(AVAILABLE_TORCH_VERSIONS.keys()) | ||
| version = max(AVAILABLE_TORCH_VERSIONS.keys(), key=Version) | ||
| warn( | ||
|
Comment on lines
360
to
362
|
||
| f"Torch Version will be selected as {version}.", | ||
|
Comment on lines
360
to
363
|
||
| stacklevel=2, | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,12 +3,11 @@ | |||||||||
|
|
||||||||||
| """Tests for installation utils.""" | ||||||||||
|
|
||||||||||
| import os | ||||||||||
| import tempfile | ||||||||||
| from pathlib import Path | ||||||||||
|
|
||||||||||
| import pytest | ||||||||||
| from pkg_resources import Requirement | ||||||||||
| from packaging.requirements import Requirement | ||||||||||
| from pytest_mock import MockerFixture | ||||||||||
|
|
||||||||||
| from anomalib.cli.utils.installation import ( | ||||||||||
|
|
@@ -34,21 +33,23 @@ def requirements_file() -> Path: | |||||||||
| def test_get_requirements(mocker: MockerFixture) -> None: | ||||||||||
| """Test that get_requirements returns the expected dictionary of requirements.""" | ||||||||||
| requirements = get_requirements("anomalib") | ||||||||||
|
|
||||||||||
| assert isinstance(requirements, dict) | ||||||||||
|
Comment on lines
35
to
37
|
||||||||||
| assert len(requirements) > 0 | ||||||||||
| for reqs in requirements.values(): | ||||||||||
| assert isinstance(reqs, list) | ||||||||||
| for req in reqs: | ||||||||||
| assert isinstance(req, Requirement) | ||||||||||
|
|
||||||||||
| mocker.patch("anomalib.cli.utils.installation.requires", return_value=None) | ||||||||||
| assert get_requirements() == {} | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def test_parse_requirements() -> None: | ||||||||||
| """Test that parse_requirements returns the expected tuple of requirements.""" | ||||||||||
| requirements = [ | ||||||||||
| Requirement.parse("torch==2.0.0"), | ||||||||||
| Requirement.parse("onnx>=1.8.1"), | ||||||||||
| Requirement("torch==2.0.0"), | ||||||||||
| Requirement("onnx>=1.8.1"), | ||||||||||
| ] | ||||||||||
| torch_req, other_reqs = parse_requirements(requirements) | ||||||||||
|
||||||||||
| torch_req, other_reqs = parse_requirements(requirements) | |
| mocker.patch("anomalib.cli.utils.installation.get_cuda_version", return_value=None) | |
| # When no package name is provided and `requires` returns None, get_requirements should return an empty dict. | |
| mocker.patch("anomalib.cli.utils.installation.requires", return_value=None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The data and logs folders sit under application/backend/ usually. But why even specify the path?
data/andlogs/should ignore all data and logs folders recursivelyThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that it then ignores all the python files in src/anomalib/data folder as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, what how about
application/**/data/application/**/logs/then?