refactor the transforms out of DiffractionObjects #184
Conversation
to make them more wiedely available in general
src/diffpy/utils/transforms.py
Outdated
| raise ValueError(invalid_q_or_wavelength_emsg) | ||
|
|
||
|
|
||
| def q_to_tth(on_q, wavelength): |
There was a problem hiding this comment.
I wonder if it might be better to have the function parameter be a Diffraction_object, and extract q and two theta from that object inside the body of the function. This would keep it consistent to the way it was before in that you can avoid having to reference instance variables in a function call. I might be wrong about this, but I don't think it's likely the functions would be used without diffraction objects, so this would be a bit of a "shorthand" compared to what it is now.
There was a problem hiding this comment.
Thanks for the suggestion @8bitsam . I chose not to do that because then you would need to instantiate a DO before you could do a transform on your data. The point of pulling it out of the DO and into transforms was so that people who weren't using DOs could still use the transforms. Perhaps it would even make sense to not require a 2D np array which is uncommon for people to have in general (they often hand around 1D vector arrays). It does seem odd to have to break up our DO and then reformulate it, which is what results from this. I think this is not a problem if there is not performance issue, because the DO is using it as a private function. My initial intention was to make it a private function because no user ever using DOs will need to call it.
There was a problem hiding this comment.
I was also thinking that maybe we shall pass in the x-array only, since we won't change anything to the intensity array in the function. It does seem to match with the DO better so maybe we should keep the 2D array.
src/diffpy/utils/transforms.py
Outdated
| return on_tth | ||
|
|
||
|
|
||
| def tth_to_q(on_tth, wavelength): |
|
|
||
| def tth_to_q(on_tth, wavelength): | ||
| r""" | ||
| Helper function to convert two-theta to q on independent variable axis. |
There was a problem hiding this comment.
It looks like we only have our tth in degrees during the conversion while radian is also an option for x-array. We should specify this somewhere around here?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #184 +/- ##
==========================================
- Coverage 99.62% 97.31% -2.31%
==========================================
Files 7 8 +1
Lines 265 261 -4
==========================================
- Hits 264 254 -10
- Misses 1 7 +6
|
3 participants
to make them more widely available in general