Skip to content
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

Merged
merged 5 commits into from
Feb 10, 2025

Conversation

tangkong
Copy link
Contributor

@tangkong tangkong commented Jan 23, 2025

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

  • Code works interactively
  • Code contains descriptive docstrings, including context and API
  • New/changed functions and methods are covered in the test suite where possible
  • Test suite passes locally
  • Test suite passes on GitHub Actions
  • Ran docs/pre-release-notes.sh and created a pre-release documentation page
  • Pre-release docs include context, functional descriptions, and contributors as appropriate

@tangkong
Copy link
Contributor Author

tangkong commented Jan 23, 2025

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

@tangkong tangkong force-pushed the bld_subpackage branch 2 times, most recently from 469b22d to 293cb7c Compare January 23, 2025 20:06
…] to cover the old install dependency group (gui+mongo)
@tangkong
Copy link
Contributor Author

A note: CI should remain unchanged as long as the .[test] dependency set remains unchanged.

@@ -0,0 +1,2 @@
PyQt5
Copy link
Member

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

Copy link
Contributor Author

@tangkong tangkong Jan 24, 2025

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)

Comment on lines +43 to +47
[tool.setuptools.dynamic.optional-dependencies.gui]
file = "gui-requirements.txt"

[tool.setuptools.dynamic.optional-dependencies.mongo]
file = "mongo-requirements.txt"
Copy link
Member

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 👍

Copy link
Member

@ZLLentz ZLLentz left a 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

@tangkong tangkong requested a review from ZLLentz January 24, 2025 00:45
@tangkong tangkong merged commit 4ca0168 into pcdshub:master Feb 10, 2025
11 checks passed
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.

BLD: consider backend specific package dependencies GUI / BLD: Make separate happi-qt sub-package?
3 participants