Skip to content

Add a dev dependency group #13264

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add a dev dependency group #13264

wants to merge 2 commits into from

Conversation

cbrnr
Copy link
Contributor

@cbrnr cbrnr commented May 27, 2025

Add a dev dependency group so that uv sync automatically installs these dependencies.

@cbrnr cbrnr requested review from larsoner and drammock as code owners May 27, 2025 07:25
@hoechenberger
Copy link
Member

hoechenberger commented May 27, 2025

When I tried this a couple of months ago, pip did not yet understand dep groups. Is this resolved now?

Also, why just dev – anything we don't want to expose in the distribution should be a dep group, including doc, no?
And the extras with that name should be nixed.

If this is not possible yet (e.g. due to missing pip support), I'd be -1 on this as it would add duplications
Unless we drop pip and go full-on uv (which IMHO is the way to go anyway, but …)

[dependency-groups]
dev = [
"mne[dev]",
"PySide6 != 6.7.0, != 6.8.0, != 6.8.0.1",
Copy link
Member

Choose a reason for hiding this comment

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

didn't we have issues in interactivity with PySide6?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure, but I'm happy to change it to PyQt6 if there are issues with PySide6. I added the latter because it is still the official Qt bindings package.

@cbrnr
Copy link
Contributor Author

cbrnr commented May 27, 2025

When I tried this a couple of months ago, pip did not yet understand dep groups. Is this resolved now?

I don't know. This change is to improve the workflow with uv.

Also, why just dev – anything we don't want to expose in the distribution should be a dep group, including doc, no?

Because dev is special because it is the default dependency group, meaning that uv sync will automatically install it.

And the extras with that name should be nixed.

This can be done later, it does not hurt to have both variants at the same time for a transition period.

If this is not possible yet (e.g. due to missing pip support), I'd be -1 on this as it would add duplications
Unless we drop pip and go full-on uv (which IMHO is the way to go anyway, but …)

I don't think that's necessary, I just wanted to make life easier for uv users.

@hoechenberger
Copy link
Member

Got you. Perhaps we should combine this with an update of our contribution docs?
Do you think we should ship a lock file, too? (probably not, since we're not an app)

Lastly, the syncing of dev by default, do you know if this is bound to stay or if there's discussions about changing this? To me, it's a bit unexpected behavior tbh (even though I appreciate it)

@cbrnr
Copy link
Contributor Author

cbrnr commented May 27, 2025

Got you. Perhaps we should combine this with an update of our contribution docs?

Sure, but we could also wait until we support uv more prominently. This is just a small quality of life change that I'd like to have rather sooner than later, so it does not have to be documented right now.

Do you think we should ship a lock file, too? (probably not, since we're not an app)

No, I wouldn't do that exactly for this reason.

Lastly, the syncing of dev by default, do you know if this is bound to stay or if there's discussions about changing this? To me, it's a bit unexpected behavior tbh (even though I appreciate it)

It's here to stay I think: https://docs.astral.sh/uv/concepts/projects/dependencies/#default-groups

@hoechenberger
Copy link
Member

Ok, you convinced me, all green from my end, then. Happy to keep PySide6 too, until we run into issues – won't be user-facing, so we can change this whenever we feel we need to.

Copy link
Member

@drammock drammock left a comment

Choose a reason for hiding this comment

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

anything we don't want to expose in the distribution should be a dep group, including doc, no? And the extras with that name should be nixed.

This can be done later, it does not hurt to have both variants at the same time for a transition period.

removing extras will cause downstream install scripts to crash, so I think we should treat as a deprecation. also, it might hurt to have both variants at the same time (emphasis added):

Tools MAY choose to provide the same or similar interfaces for interacting with Dependency Groups as they do for managing extras. Tools authors are advised that the specification does not forbid having an extra whose name matches a Dependency Group. Separately, users are advised to avoid creating Dependency Groups whose names match extras, and tools MAY treat such matching as an error.

@@ -2,6 +2,12 @@
build-backend = "hatchling.build"
requires = ["hatch-vcs", "hatchling"]

[dependency-groups]
dev = [
"mne[dev]",
Copy link
Member

Choose a reason for hiding this comment

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

I believe to really get everything one needs to do mne[full-pyside6,test-extra,dev] (or sub full-pyside6 for full to get PyQt6)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just like with extras, we can add as many dependency groups as we like. I'd say that really everything should probably not be assigned to dev, but maybe dev-full or something like that. A more lightweight dev setup should probably also be supported by the test dependency group.

Regarding the warning on duplicate extras and dependency group names, uv does not have any issue with it. pip does not support dependency groups AFAIK, so we should be good.

@drammock
Copy link
Member

drammock commented Jun 6, 2025

it might hurt to have both variants at the same time (emphasis added):

Tools MAY choose to provide the same or similar interfaces for interacting with Dependency Groups as they do for managing extras. Tools authors are advised that the specification does not forbid having an extra whose name matches a Dependency Group. Separately, users are advised to avoid creating Dependency Groups whose names match extras, and tools MAY treat such matching as an error.

Regarding this point, can someone check how pip, pixi, and uv each handle the case where a dependency group name exactly matches an extras name? If those three all handle it the same way (and it's not "raise an error") then I'd feel fine moving forward with this duplication of dev.

Failing that: we could consider skipping/shortening the deprecation period for the dev extras, since it's not exactly user-facing (set of people impacted by it should be much smaller than "our whole user base")

@cbrnr
Copy link
Contributor Author

cbrnr commented Jun 8, 2025

This works with uv and pip (which does not support dependency groups at all). I have not tested with pixi since I don't use it.

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.

3 participants