make alphamap optional and update installation instructions#121
make alphamap optional and update installation instructions#121
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes the alphamap dependency optional for alphaquant to reduce the default dependency footprint. Users can now install alphaquant without alphamap and only add it when needed for sequence visualization features.
Changes:
- Removed
alphamapfrom core requirements and created separate optional dependency groups - Added runtime import guards and helpful error messages when
alphamapfunctionality is accessed without the package installed - Updated CI/CD workflows to include the
alphamapextra in test installations
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| requirements/requirements_loose.txt | Removed alphamap from core dependencies |
| requirements/requirements_alphamap_loose.txt | Created new optional dependency file for unpinned alphamap |
| requirements/requirements_alphamap.txt | Created new optional dependency file for pinned alphamap version |
| requirements/requirements.txt | Removed alphamap from pinned core dependencies |
| pyproject.toml | Added alphamap and alphamap-stable optional dependency groups |
| alphaquant/ui/dashboard_parts_plots_proteoforms.py | Added ImportError handling for missing alphamap with installation instructions |
| alphaquant/plotting/fcviz.py | Added conditional import with HAS_ALPHAMAP flag and ImportError guard |
| alphaquant/plotting/alphamapviz.py | Added conditional imports, warning, and ImportError guards for both visualizer classes |
| README.md | Added installation instructions for alphamap optional extras |
| .github/workflows/run_example_nbs.yml | Added alphamap to CI test installation |
| .github/workflows/install_and_unit_tests_multiple_platforms.yml | Added alphamap to CI test installations |
| .github/workflows/install_and_unit_tests.yml | Added alphamap to CI test installation |
| .github/workflows/e2e_tests_quick_multiple_platforms.yml | Added alphamap to CI test installations |
| .github/workflows/e2e_tests_quick.yml | Added alphamap to CI test installation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not HAS_ALPHAMAP: | ||
| raise ImportError( | ||
| "alphamap is required for get_pyteomics_fasta. " | ||
| "Install it with: pip install \"alphaquant[alphamap]\"" | ||
| ) | ||
| return alphamap.organisms_data.import_fasta(organism) |
There was a problem hiding this comment.
The indentation level suggests this is inside a function body, but Python typically uses 4 spaces per level. This line appears to be indented with 8 spaces when it should likely be 4 spaces to align with the function definition at line 205.
| if not HAS_ALPHAMAP: | |
| raise ImportError( | |
| "alphamap is required for get_pyteomics_fasta. " | |
| "Install it with: pip install \"alphaquant[alphamap]\"" | |
| ) | |
| return alphamap.organisms_data.import_fasta(organism) | |
| if not HAS_ALPHAMAP: | |
| raise ImportError( | |
| "alphamap is required for get_pyteomics_fasta. " | |
| "Install it with: pip install \"alphaquant[alphamap]\"" | |
| ) | |
| return alphamap.organisms_data.import_fasta(organism) |
alphaquant/plotting/alphamapviz.py
Outdated
| HAS_ALPHAMAP = True | ||
| except ModuleNotFoundError: | ||
| warnings.warn( | ||
| "Dependency 'alphamap' not installed. If you want to use its functionality, install alphaquant with the 'alphamap' extra." |
There was a problem hiding this comment.
The warning message should include installation instructions for consistency with the ImportError messages used elsewhere in the codebase. Consider changing to: "Dependency 'alphamap' not installed. Install it with: pip install \"alphaquant[alphamap]\""
| "Dependency 'alphamap' not installed. If you want to use its functionality, install alphaquant with the 'alphamap' extra." | |
| "Dependency 'alphamap' not installed. Install it with: pip install \"alphaquant[alphamap]\"" |
66b0c73 to
8d825cb
Compare
b204f52 to
bc514f5
Compare
ammarcsj
left a comment
There was a problem hiding this comment.
Hey, thx for looking into this. The sequence visualisation is an essential feature in case the GUI is installed, so if we take alphamap out for the default pip install, my intuition is that we should recommend the pip install with alphamap and fixed dependencies as the recommended way of pip installing alphaquant and GUI (marked it below)
Did you by any chance check wether the gui works with this new way of handling dependencies?
| if you want to add the GUI to your environment, you can install it with the following command: | ||
|
|
||
| ```bash | ||
| pip install "alphaquant[stable,gui-stable]" |
There was a problem hiding this comment.
I think we should then add the alphamap dependency also to this example call and say that they can leave it away if they don't need it
There was a problem hiding this comment.
Hey,
thanks for pointing out that this is a gui-only feature. I could also just move the alphamap dependency to the gui extra then (thus simplifying installation workflow)?
Also, I did check, the gui works without alphamap installed.
There was a problem hiding this comment.
(merged the two extras, much simpler now :-) )
Motivation: have slimmer dependencies for alphapeptools