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
DOC: Answer two common questions. #1223
DOC: Answer two common questions. #1223
Conversation
01daaab
to
653a0a8
Compare
653a0a8
to
615948b
Compare
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.
Nice work! Left a few comments with suggestions.
Fix typos. Co-Authored-By: Maksim Rakitin <mrakitin@users.noreply.github.com> Co-Authored-By: Teddy Rendahl <trendahl@slac.stanford.edu>
|
||
.. code-block:: python | ||
|
||
det.exposure_time.set(3) |
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 am wary of this code block because it will be copy-pasted into a plan.
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 recommend either setting the time-related parameter(s) in advance: | ||
|
||
.. code-block:: python |
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 agree with Tom. Suggest replacing lines 124-128 with this:
We recommend either setting the time-related parameter(s) in advance.
For interactive use:
.. code-block:: python
det.exposure_time.set(3)
From a plan:
.. code-block:: python
yield from bluesky.plan_stubs.mv(det.exposure_time, 3)
Modern CCD detectors typically parametrize exposure time with *multiple* | ||
parameters. There is no one "exposure time" that can be applied to all | ||
detectors. | ||
|
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.
Perfect place to expand on why a common method is a challenge. Add this paragraph:
Scalers also parametrize exposure time with *multiple*
parameters. The most common term (used by
`EpicsScaler` and `ScalerCH`) is `det.preset_time`.
Do multi-channel analyzers and multi-channel scalers set the time differently. This is the place to mention briefly.
This is a surprising test failure:
|
Hm, also seeing this failure on other recent PRs. Very strange. |
OK, problem identified and fixed in #1225. Unrelated to this change. |
Answer two common questions.
Rendered here for easier reading: