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

Breaking: merge instrument and rig, make rig accept components: List[Union[]] #1238

Draft
wants to merge 35 commits into
base: release-v2.0.0
Choose a base branch
from

Conversation

dbirman
Copy link
Member

@dbirman dbirman commented Jan 14, 2025

This PR merges the instrument.py and rig.py files together and sets up Rig to accept a List[Union[]] of components instead of specifying parameters to accept. Unfortunately it was best to do these together rather than separately, which makes this a very large PR.

To allow the Union to work properly the Assembly classes needed to have a device_type field added, even though they don't inherit from Device itself. The code throws warnings if you try to add a new option to the union that doesn't include the discriminator field, so there's no need to add a validator.

I discovered a huge number of tests that were mis-specified during this refactor, I've simplified testing so that instrument now properly tests its own validators (rather than being sprinkled in other files like test_imaging).

This PR got bloated... it now also:

  • Deprecates platform

@dbirman dbirman linked an issue Jan 14, 2025 that may be closed by this pull request
2 tasks
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.0] Combine rig and instrument
1 participant