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

ENH: New devices #400

Merged
merged 34 commits into from
Apr 14, 2020
Merged

ENH: New devices #400

merged 34 commits into from
Apr 14, 2020

Conversation

ZryletTC
Copy link
Contributor

@ZryletTC ZryletTC commented Mar 20, 2020

Description

Creates new classes for goniometers, von Hamos spectrometers, Beckhoff liquid jets, TimeTools, and PFLSes. Adds CombinedInOutRecordPositioner for use with X/Y combined state devices.
Also included minor other fixes.

Motivation and Context

Need support for these devices in new python.
Also fixes #384.

How Has This Been Tested?

Created tests for new devices. All tests pass.

Where Has This Been Documented?

Added docstrings. Their style will be fixed in upcoming PR.

@codecov-io
Copy link

codecov-io commented Mar 20, 2020

Codecov Report

Merging #400 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #400   +/-   ##
=======================================
  Coverage   93.82%   93.82%           
=======================================
  Files          40       40           
  Lines        3561     3561           
=======================================
  Hits         3341     3341           
  Misses        220      220           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ba6ccd...5ba6ccd. Read the comment docs.

@ZryletTC ZryletTC force-pushed the newdevices branch 2 times, most recently from 42a6a83 to d801c22 Compare March 20, 2020 23:45
@ZryletTC
Copy link
Contributor Author

Merged in pre-commit changes.

@ZryletTC ZryletTC self-assigned this Mar 26, 2020
@ZryletTC ZryletTC marked this pull request as ready for review April 10, 2020 02:12
@ZryletTC
Copy link
Contributor Author

I decided to make the docstring style consistent in the rest of the repo so I'll add that as a separate PR because there a lot of changes. Style changes for these files will be included there.

@ZryletTC
Copy link
Contributor Author

ZryletTC commented Apr 10, 2020

Docs aren't building, please don't merge until fixed.

  • Could be fixed by m2r#55
  • If desired, can be bypassed for now by pinning sphinx<3.0.0 or specifying the hotfix branch of m2r
  • Realistically, that PR will never be merged as the repo has been inactive since 2018 so a better course of action would probably be to drop m2r and switch to recommonmark as others are

@ZryletTC ZryletTC mentioned this pull request Apr 10, 2020
pcdsdevices/ipm.py Outdated Show resolved Hide resolved
pcdsdevices/ipm.py Show resolved Hide resolved
pcdsdevices/pim.py Show resolved Hide resolved
pcdsdevices/gon.py Outdated Show resolved Hide resolved
pcdsdevices/jet.py Show resolved Hide resolved
pcdsdevices/lens.py Show resolved Hide resolved
@ZLLentz
Copy link
Member

ZLLentz commented Apr 10, 2020

You don't have to fix the docs in this PR, we can do that in a separate PR.

pcdsdevices/tt.py Outdated Show resolved Hide resolved
@ZryletTC ZryletTC linked an issue Apr 10, 2020 that may be closed by this pull request
@ZryletTC
Copy link
Contributor Author

Doc build fix merged in.

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.

I don't have much to add. Thanks for fixing all the side things you spotted while making this PR. Make sure you change the title of this PR to something other than WIP before we merge.

Copy link
Contributor

@silkenelson silkenelson left a comment

Choose a reason for hiding this comment

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

I think the XCS goniometer is now missing its XYZ motors. As they are not in base anymore, the "withArm" goniometer needs an XYZ stage object attached to it.

I couldn't figure out how to reply below this....
From staring at setup photos that Rajan sent to me, it looks one can remove the XYZ to fit vacuum chambers. So leave the code as it is and we see how to create the XYZ if it exists for XCS later.

@ZryletTC ZryletTC changed the title WIP: New devices ENH: New devices Apr 14, 2020
@ZryletTC
Copy link
Contributor Author

ZryletTC commented Apr 14, 2020

I think the XCS goniometer is now missing its XYZ motors. As they are not in base anymore, the "withArm" goniometer needs an XYZ stage object attached to it.

@silkenelson, is the XYZ stage not removable in XCS? If it is, I'll add them in my implementation of #418.
If not, I can add an inheritance now.

@ZryletTC
Copy link
Contributor Author

Raj showed me a time when the XYZ stage was removed for an experiment so I will add them as with #418.

@ZryletTC ZryletTC merged commit ee106ac into pcdshub:master Apr 14, 2020
@ZryletTC ZryletTC deleted the newdevices branch April 14, 2020 19:40
@klauer
Copy link
Contributor

klauer commented Apr 14, 2020

🎉

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.

Error from pcds-envs test BaseInterface needs to be added as parent of ipm.py classes
6 participants