Move types to subpackages#607
Conversation
github-actions
Bot
added
essdiffraction
|
Hi! Your PR was missing some labels 🔖 so I added them: essdiffraction essreduce essreflectometry esssans essspectroscopy |
|
Depends on how close that PR is to merging? Resolving conflicts should be simple |
|
My guess is that it will still take a while to figure everything out correctly in that PR. |
| """Incident monitor""" | ||
| TransmissionMonitor = NewType('TransmissionMonitor', int) | ||
| """Transmission monitor""" | ||
| FrameMonitor0 = NewType('FrameMonitor0', int) |
There was a problem hiding this comment.
Would there be any use in providing a default set of monitors? (Monitor1-4?)
There was a problem hiding this comment.
I don't think so. We shouldn't use them in workflows because they carry no meaning. So I think they would only be good for testing, and then we can easily define them in the tests.
| like ``SampleRun`` or ``BackgroundRun``. | ||
| """ | ||
|
|
||
| SampleRun = NewType('SampleRun', int) |
There was a problem hiding this comment.
Why is the SampleRun special?
Shouldn't we remove all in the future, and sub-packages can decide if they want to name it SampleRun or something else?
Is there any code internally to essreduce that makes use of SampleRun or is it only in the tests/docs?
There was a problem hiding this comment.
It is special in how I and maybe we think about workflows. But that is not a good argument on its own.
There are some uses in live and polarization. Removing the ones in live might be tricky. But maybe that code should be in esslivedata anyway? And for polarization, there is this: #615
There was a problem hiding this comment.
I would have thought that everything in essreduce could just be RunType, unless I am forgetting something?
But maybe that code should be in esslivedata anyway?
Yes, I would vote to move it there.
2 participants
First step of #70