Skip to content

Install packages together#37

Merged
mbercx merged 1 commit intoaiidateam:mainfrom
danielhollas:install-deps-together
Oct 1, 2025
Merged

Install packages together#37
mbercx merged 1 commit intoaiidateam:mainfrom
danielhollas:install-deps-together

Conversation

@danielhollas
Copy link
Collaborator

@danielhollas danielhollas commented Sep 30, 2025

Implementation of #30 (comment)

@danielhollas
Copy link
Collaborator Author

@mbercx LMK if you agree with the direction of this PR. I quite like how this turned out to be a big code simplification, without needing to remove the "install additional plugin" features.

@danielhollas danielhollas requested a review from mbercx September 30, 2025 02:16
@danielhollas danielhollas changed the title WIP: Install packages together Install packages together Sep 30, 2025
@danielhollas danielhollas force-pushed the install-deps-together branch 2 times, most recently from f76f0c6 to f46d3ac Compare September 30, 2025 02:48
@danielhollas danielhollas marked this pull request as ready for review September 30, 2025 02:49
Copy link
Member

@mbercx mbercx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @danielhollas! I left some small fixes and a question, but I like the code simplification a lot. Note that you are also removing the (actually broken, see #38) feature of installing from a GitHub. I'm fine with removing it for sure, we can always see if we want to reintroduce it in the future.

@danielhollas
Copy link
Collaborator Author

Okay, I added an error handling for the install command, here's how it looks now:

image

This is good to go from my side, @mbercx feel free to adjust and fix commits as you please, it's getting late here. 😅

@danielhollas
Copy link
Collaborator Author

Note: The error handling needs to be added to all subprocess.run calls (this is actually what ruff was flagging in the pre-commit PR). I'll do the rest in a separate PR.

@danielhollas
Copy link
Collaborator Author

Note that you are also removing the (actually broken, see #38) feature of installing from a GitHub. I'm fine with removing it for sure, we can always see if we want to reintroduce it in the future.

Yes. To be precise, I am removing the ability to have a cloned local repo installed in an editable mode. You can still install packages directly from github (as demonstrated by the added test in the CI).

@mbercx
Copy link
Member

mbercx commented Oct 1, 2025

Yes. To be precise, I am removing the ability to have a cloned local repo installed in an editable mode. You can still install packages directly from github (as demonstrated by the added test in the CI).

Correct, apologies for the careless language, this is what I meant. We'll see later if we want this feature to be reintroduced (and working this time 🙈).

This is good to go from my side, @mbercx feel free to adjust and fix commits as you please, it's getting late here. 😅

Alright, thanks again @danielhollas! I'll adapt and merge then 🙏

The `--plugin` option of the `create` command allows the user to install additional
packages next to `aiida-core`. In the current approach these are installed one by one
after completing the installation of `aiida-core`. Moreover, if any of the package
installations failed, this would do so silently.

Here we improve the installation step by installing all packages in one go, which
makes it easier for the resolver to take into account all constraints. Additionally,
any errors are now caught and shown to the user.

This commit also removes the broken feature to clone packages from their GitHub
repository and install them locally. Note that installing packages from GitHub is still
possible using the typically `pip` syntax `git+`:

    aiida-project create test -p git+https://github.com/aiidateam/aiida-quantumespresso

The repo is simply no longer cloned to the `git` directory`. This directory is also
removed from the default project layout, since it was confusing to refer to it due to
the similarity with the `.git` directory.

Co-authored-by: Daniel Hollas <daniel.hollas@bristol.ac.uk>
@mbercx mbercx force-pushed the install-deps-together branch from 2bf3b83 to 5fb8750 Compare October 1, 2025 00:38
@mbercx
Copy link
Member

mbercx commented Oct 1, 2025

Thanks again @danielhollas, great work as always. 🙌

@mbercx mbercx merged commit da3b72b into aiidateam:main Oct 1, 2025
6 checks passed
@danielhollas danielhollas deleted the install-deps-together branch October 1, 2025 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants