Espnet3/install only #6139
Espnet3/install only #6139
Conversation
- Included only installation-related script. - Original commit message: Refactor Docker setup and CI configuration: update Dockerfile to use ARG for base image, streamline test scripts, and enhance .gitignore; add build image script and devcontainer configuration for Debian CI.
- I only included devcontainer document. - original message: Refactor Dockerfile user creation logic for improved readability; add devcontainer documentation and images for better guidance.
…age pyproject.toml during installation
|
@Fhrozen Could you review this PR, and if there is any missing files please let me know!! |
for more information, see https://pre-commit.ci
|
@Masao-Someki Under review. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Thank you @Fhrozen and I didn't know that we had the copilot review! I'll definitely need that in my future PRs! |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
for more information, see https://pre-commit.ci
… into espnet3/install_only
| try: | ||
| import hydra | ||
| from omegaconf import DictConfig, ListConfig | ||
| except ImportError: | ||
| import logging | ||
|
|
||
| logging.warning( | ||
| "If you are running state-space model," "run `pip install espnet['task-asr']`." | ||
| ) |
There was a problem hiding this comment.
Is it correct?
hydra and omegaconf are related to espnet-3.
Why do they appear here?
What we need to install task-asr
There was a problem hiding this comment.
It looks like the files under espnet2/asr/state_spaces/ use Hydra and OmegaConf.
Also, opt_einsum is only used within this directory, so I’ve included these in the task-asr group...
|
@Masao-Someki and @Fhrozen, this might help To reflect flake8 in the Here’s how you can do it:
Summary:
Would you like to see the current contents of your |
pyproject.toml
Outdated
| all = [ | ||
| "espnet[task-asr]", | ||
| "espnet[task-tts]", | ||
| "espnet[task-enh]", | ||
| "espnet[task-spk]", | ||
| ] |
There was a problem hiding this comment.
all does not seem to include all.
This will also cause some issues in the future when someone extends the task.
They may forget to add it to all (like you do).
Is there a way to include all tasks by using task* or something like that?
There was a problem hiding this comment.
From Gemini:
That is an excellent and insightful question that explores the boundaries of what pyproject.toml is designed to do.
The short answer is no, it is not possible to use wildcards (task-*) or loops directly within the pyproject.toml file to dynamically group dependencies.
The Reason: pyproject.toml is Declarative, Not Programmatic
The `pyproject.toml file is designed to be a declarative and static configuration file. This means you declare what your project's configuration is, but you cannot include logic—like loops, conditionals, or wildcards—to compute that configuration. Its strength lies in its simplicity and predictability; any tool can read the file and know the project's metadata without executing any code.
The Standard Way: Explicitly Listing the Extras
The correct and standard way to achieve your goal is to explicitly list each extra you want to include in your my-packages group. While it may feel repetitive, it is unambiguous and the officially supported method.
# pyproject.toml
[build-system]
requires = ["setuptools>=61.0"]
build-backend = "setuptools.build_meta"
[project]
name = "my-component-package"
version = "1.0.0"
# 1. Define your individual component extras
[project.optional-dependencies]
task-1 = [
"pandas",
]
task-2 = [
"matplotlib",
"seaborn"
]
task-3 = [
"scikit-learn"
]
# ...and so on for task-4, task-5, etc.
# 2. Create the group that explicitly includes the others
all-tasks = [
"my-component-package[task-1]",
"my-component-package[task-2]",
"my-component-package[task-3]",
]
# You might also want a group for development tools
dev = [
"my-component-package[all-tasks]", # Include all the above
"pytest",
"ruff"
]The Advanced Way: Adding Dynamic Logic with setup.py
If you absolutely must have dynamic logic (for example, in a very complex project with dozens of auto-generated extras), you can fall back to using a setup.py script.
This involves telling pyproject.toml that the optional-dependencies are "dynamic," meaning they will be determined at build time by the build backend (setuptools) running the setup.py script.
This adds complexity and is generally not recommended unless you have a strong reason.
Conclusion
| Method | How It Works | Pros | Cons |
|---|---|---|---|
Standard (pyproject.toml only) |
Explicitly list each extra in the group. | Simple, clear, standard, easy for tools to analyze. | Requires manual updating if you add a new task-N. |
Advanced (Dynamic setup.py) |
Use Python code to generate the list of extras. | Automated, powerful for complex scenarios. | Adds complexity, dependencies are not statically visible, can be harder to debug. |
There was a problem hiding this comment.
@Fhrozen Could you try copilot review? If it can detect this error, we can continue with that.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Okay, it seems we need additional test that checks |
tools/Makefile
Outdated
| @@ -128,7 +128,9 @@ lightning.done: pytorch.done | |||
| # NOTE(kamo): conda_packages is not necessary for installation of espnet, but add it the dependencies just in case. | |||
| espnet.done: pytorch.done conda_packages.done lightning.done | |||
| @echo NUMPY_VERSION=$(shell . ./activate_python.sh && python3 -c "import numpy; print(numpy.__version__)") | |||
| cp ./setup.py ../ && mv ../pyproject.toml ../pyproject.toml.tmp | |||
There was a problem hiding this comment.
Pull Request Overview
This PR restructures the ESPnet installation process by splitting the dependencies into optional groups, allowing users to install only the required extras (e.g., asr, tts, all). Key changes include updates to setup.py, Makefile, and pyproject.toml for dependency management; additions such as the uninstall_extra.py script; and modifications in task and test modules to support lazy dependency imports.
Reviewed Changes
Copilot reviewed 26 out of 29 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tools/setup.py | Updated package discovery and test dependency version ranges. |
| tools/Makefile | Added steps to temporarily move configuration files during installation. |
| test_utils/uninstall_extra.py | Introduced a helper script to uninstall extra dependencies. |
| test/espnetez/test_integration_espnetez_ft.py | Revised lazy imports and dependency management for inference classes. |
| test/espnet2/text/test_phoneme_tokenizer.py | Adjusted dependency imports to handle missing packages gracefully. |
| pyproject.toml | Split dependencies into optional groups and refined version ranges. |
| espnetez/task.py | Converted static TASK_CLASSES to lazy imports via get_task_class. |
| espnet2/asr/frontend/asteroid_frontend.py | Changed to lazy import asteroid_filterbanks and added an error if missing. |
| (Other files in tests, CI, and docs) | Various adjustments to installation instructions and CI workflows for ESPnet3. |
Comments suppressed due to low confidence (2)
espnet2/asr/frontend/asteroid_frontend.py:53
- The error message in the except clause suggests installing espnet['task-spk'] even though this module belongs to ASR (front-end). Please verify whether the correct dependency group (e.g., task-asr) should be recommended.
except ImportError:
espnet2/text/phoneme_tokenizer.py:259
- The runtime error message for a missing 'g2p_en' dependency appears to have inconsistent formatting (e.g. unbalanced quotation/backticks). Consider revising the message for clarity and consistency.
raise RuntimeError(
What?
Update installation of espnet. With this PR, we can install espnet in a structured way.
E.g.,
pip install espnet[all],pip install espnet[core,task-asr]Why?
The dependencies of espnet becomes very large. We wanted to split it.
See also
#6133
#6135
Note
This PR might include formatting automatically done by precommit.ci, but that will be merged to the master branch in another PR. This PR should only be merged after that PR.