generated from tweag/project
-
Notifications
You must be signed in to change notification settings - Fork 14
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
Initial support for environment.yml files #457
Draft
jherland
wants to merge
7
commits into
jherland/pixi-toml-support
Choose a base branch
from
jherland/environment-yml-support
base: jherland/pixi-toml-support
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Initial support for environment.yml files #457
jherland
wants to merge
7
commits into
jherland/pixi-toml-support
from
jherland/environment-yml-support
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jherland
force-pushed
the
jherland/environment-yml-support
branch
2 times, most recently
from
September 18, 2024 12:49
c6cca06
to
5e5455e
Compare
jherland
force-pushed
the
jherland/pixi-toml-support
branch
from
September 19, 2024 08:32
2de6d56
to
88c9ba0
Compare
jherland
force-pushed
the
jherland/environment-yml-support
branch
3 times, most recently
from
September 19, 2024 09:12
c5325d9
to
7c2b9a2
Compare
jherland
force-pushed
the
jherland/pixi-toml-support
branch
from
September 19, 2024 10:16
a72e423
to
02c28ba
Compare
jherland
force-pushed
the
jherland/environment-yml-support
branch
2 times, most recently
from
September 19, 2024 11:11
9f73f59
to
c70fc36
Compare
jherland
force-pushed
the
jherland/pixi-toml-support
branch
from
September 19, 2024 11:28
4ede02f
to
de059c7
Compare
jherland
force-pushed
the
jherland/environment-yml-support
branch
from
September 19, 2024 11:28
c70fc36
to
f5460cb
Compare
jherland
force-pushed
the
jherland/pixi-toml-support
branch
from
September 19, 2024 11:55
de059c7
to
838231f
Compare
jherland
force-pushed
the
jherland/environment-yml-support
branch
from
September 19, 2024 11:55
f5460cb
to
63dd427
Compare
jherland
force-pushed
the
jherland/pixi-toml-support
branch
from
September 19, 2024 12:59
838231f
to
fc39cf4
Compare
jherland
force-pushed
the
jherland/environment-yml-support
branch
from
September 19, 2024 12:59
63dd427
to
2ed9c81
Compare
jherland
force-pushed
the
jherland/pixi-toml-support
branch
from
September 19, 2024 13:13
fc39cf4
to
4da7004
Compare
jherland
force-pushed
the
jherland/environment-yml-support
branch
from
September 19, 2024 13:14
2ed9c81
to
aa5ca07
Compare
jherland
force-pushed
the
jherland/pixi-toml-support
branch
from
September 19, 2024 13:57
4da7004
to
09e43d8
Compare
jherland
force-pushed
the
jherland/environment-yml-support
branch
from
September 19, 2024 13:57
aa5ca07
to
d89f6bd
Compare
jherland
force-pushed
the
jherland/pixi-toml-support
branch
from
September 19, 2024 15:10
09e43d8
to
d76d45e
Compare
jherland
force-pushed
the
jherland/environment-yml-support
branch
from
September 19, 2024 15:10
d89f6bd
to
ea9e35d
Compare
fawltydeps/extract_declared_dependencies.py has been growing longer, and is about to grow even longer with the introduction of parsing environment.yml files. Now is the time to split this up into multiple modules that each focus on one particular (group of) dependenciy declaration formats. Also, take the opportunity to rename extract_declared_depenedencies into extract_deps: We use the term "deps" everywhere (including in our project name), so there is no need to spell out "dependencies" in all of these filenames. No functional changes in this commit. It's all just moving code between files, and fixing up associated references accordingly.
This is needed to start parsing environment.yml files from the Conda ecosystem. v6.0.1 is the last version to support Python v3.7, so this is our minmum version for now. Adding a yaml dependency to FawltyDeps makes it available inside the Python environment that is used by the packages.SysPathPackageResolver. This changes the behavior of a couple of real_projects tests, where we previously expected yaml/PyYAML to be reported as undeclared/unused when no Python environment with PyYAML installed was present. Now, we can assume that PyYAML -> yaml is always correctly mapped via sys.path. Also, a potential future issue to be solved is made apparent by our need to declare types-PyYAML in ignore_unused: I suspect the "types-$PACKAGE" form is quite common, and should be handled in the same way as "-stubs" suffixes inside Package.has_type_stubs().
Include support for parsing Pip dependencies embedded within environment.yml files, like this: ```yaml name: example dependencies: - jupyterlab=1.0 - pip=19.1 - pip: - kaggle==1.5 - yellowbrick==0.9 ``` Also add test cases, mirroring the structure we already use to test our pyproject.toml and pixi.toml parsers.
In tests/test_deps_parser_determination.py, test_explicit_parse_strategy__mismatch_yields_appropriate_logging() tests what happens when the --deps-parser-choice option is (ab)used to parse a deps file - e.g. setup.cfg - with the "wrong" parser - e.g. parse_setup_py(). The test verifies that we print a warning message when such mismatch occurs. However, the test also _assumes_ that when no mismatch is present, the empty file being passed to the appropriate parser would yield zero warning messages. Although this happens to be true for our existing parsers, we are about to add a new parser for environment.yml files, where an empty file is _not_ silently accepted, but rather produces this error message: Failed to parse path/to/environment.yml: No top-level mapping found! I believe the implicit assumption in this test is wrong: an empty file could very well produce warnings/errors, and we should instead make sure that the files being passed in the test are actually formatted according to their filenames (e.g. a setup.cfg file should be formatted as a valid setup.cfg file). Fortunately, we already have the fake_project() fixture which is able to produce valid deps file in all our supported formats, so this is fairly straightforward to fix. After fixing this, however, I discover a new issue: When passing e.g. a valid setup.cfg file to the parse_setup_py() function, it (obviously, in hindsight) fails with a SyntaxError: parse_setup_py() expects to find valid Python code, and the setup.cfg format is certainly not that. (The same goes for other file formats passed to parse_setup_py().) This SyntaxError is NOT caught by the parser, and causes FawltyDeps to abort. The fix is straightforward - catch the exception and print an error mesage - and the end result is that we now subject our parsers to a bit more scrutiny: - When there is a mismatch between the chosen parser and the given file, we verify that a warning about this mismatch is printed (along with any other warnings/errors that result from applying the "wrong" parser to this file). - When there is no mismatch between the chosen parser and the given file, we verify that a correctly formatted file (courtesy of fake_project()) generates no warnings/errors at all.
Now that we can parse dependency declarations from environment.yml files, we need to expose this functionality in our docs + CLI, as well as automatically find environment.yml files while traversing projects.
This was created with the following commands: - conda create --name conda_example python=3.12 - conda activate conda_example - conda install requests - conda env export --from-history > environment.yml - echo "import requests" > main.py followed by copying the corresponding Conda environment from ~/.conda/envs/conda_example into this sample project directory, and then removing/truncating files that I do not currently consider to be important for FawltyDeps to make sense of this project. The result is a simple Conda project with two declared Conda dependencies (Python itself + requests), along with a single source file that imports requests. The expected result of running FawltyDeps here is to report no undeclared or unused dependencies. Please note that since Conda puts environments under ~/.conda, and NOT inside the project directory, this example is somewhat contrived: We can not expect the Conda environment to be automatically discovered by FawltyDeps (unless FD itself is installed and running from there), but we must instead rely on the use of --pyenv to point to ~/.conda/envs/conda_example (in the general case, or tests/sample_project/conda_example/.conda/envs/conda_example in this test case)
jherland
force-pushed
the
jherland/pixi-toml-support
branch
from
September 20, 2024 08:32
d76d45e
to
8198e99
Compare
jherland
force-pushed
the
jherland/environment-yml-support
branch
from
September 20, 2024 08:33
ea9e35d
to
466baa9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds support for Conda projects that use environment.yml for declaring dependencies.
NOTE: The same Conda caveat as for Pixi still applies: We do not currently differentiate between Conda dependencies and PyPI dependencies, meaning that we assume that a Conda dependency named FOO will map one-to-one to a Python package named FOO. This is certainly not true for Conda dependencies that are not Python packages, and it probably isn't even true for all Conda dependencies that do indeed include Python packages.
Commits:
environment.yml
filesenvironment.yml
support to the rest of FawltyDepssample_projects
: Add example Conda projecttest_traverse_project
: Add tests using the Conda sample project