-
Notifications
You must be signed in to change notification settings - Fork 29
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
BLD: split ui dependencies into subpackage #353
Conversation
In the lightpath, we only adjusted the pip builds in the repo, and left the conda outputs for the feedstock. I'm going to verify that the pip builds work here, then try applying the same conda output recipe format to the conda recipe here. I think it'd be nice to have a build check independent of conda-forge, but I'm not sure if any of the output setup is conda-forge specific Edit: conda multi-output builds were not happy |
469b22d
to
293cb7c
Compare
…] to cover the old install dependency group (gui+mongo)
A note: CI should remain unchanged as long as the |
@@ -0,0 +1,2 @@ | |||
PyQt5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's at least some potential consideration for not having PyQt5 here, I don't remember best practices for qtpy apps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We include PyQt5
for both atef
and lightpath
, which I was looking at for reference. You're right that qtpy
is supposed to be our qt-agnostic translation layer, though without it pip won't install PyQt5. (at least from what I can verify from my testing)
[tool.setuptools.dynamic.optional-dependencies.gui] | ||
file = "gui-requirements.txt" | ||
|
||
[tool.setuptools.dynamic.optional-dependencies.mongo] | ||
file = "mongo-requirements.txt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this split is pretty reasonable 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable
Missing release notes
Description
Splits happi gui dependencies into subpackages (pip) or separate outputs (conda)
Motivation and Context
closes #347
closes #340
How Has This Been Tested?
Added some ui tests, but mostly relying on CI here to assess failure
Where Has This Been Documented?
This PR
Pre-merge checklist
docs/pre-release-notes.sh
and created a pre-release documentation page