Add reader for Jobin-yvon .xml-format #25
Conversation
pietsjoh
changed the title
Codecov ReportBase: 82.87% // Head: 83.37% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #25 +/- ##
==========================================
+ Coverage 82.87% 83.37% +0.50%
==========================================
Files 40 40
Lines 8041 8337 +296
Branches 1860 1930 +70
==========================================
+ Hits 6664 6951 +287
+ Misses 913 911 -2
- Partials 464 475 +11
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
@LMSC-NTappy as you have implemented the |
rsciio/jobin_yvon/api.py
Outdated
| name: Str | ||
| Name of the axis. | ||
| """ | ||
| scale = np.abs(array[0] - array[-1]) / (array.size - 1) |
There was a problem hiding this comment.
I like the approach of loading as uniform axis and warn the user of possible non-uniformity.
However, I would use a regression method such as numpy.polyfit in order to minimize the linearized axis error:
[scale, offset] = np.polyfit(np.arange(array.size),array,1)
There was a problem hiding this comment.
An advantage of the method I used is that the boundaries match exactly.
Moreover, the offset obtained from the fit is wrong when the axis contains negative values.
For example there is a test file with navigation axis (-2, 0, 2).
I personally wouldn't use the fit for the offset calculation, but I could imagine using a fit for the scale.
There was a problem hiding this comment.
Indeed, the only additional degree of freedom of the fit should be that the first and last point are not fixed. So if we want to keep their positions, there is no advantage of the fit.
There was a problem hiding this comment.
An advantage of the method I used is that the boundaries match exactly.
Indeed, the only additional degree of freedom of the fit should be that the first and last point are not fixed. So if we want to keep their positions, there is no advantage of the fit.
I don't see any virtue in ensuring that first/last points match exactly but if that's a concern to you I guess you have good reasons.
Moreover, the offset obtained from the fit is wrong when the axis contains negative values.
For example there is a test file with navigation axis (-2, 0, 2).
Sounds odd to me. Running this gives
np.polyfit(x=np.arange(3),y=[-2,0,2],deg=1)
#out: array([ 2., -2.])Which is exactly a scale of 2 and an offset of -2
I personally wouldn't use the fit for the offset calculation, but I could imagine using a fit for the scale.
Definitely this would induce more error than what is currently implemented (at lease in some cases)
You do as you please :)
There was a problem hiding this comment.
Which is exactly a scale of 2 and an offset of -2
I made a mistake here. Yes it should also work in this case.
I use a fit now to get the scale/offset of the signal axis.
Empad and Jobin Yvon share the same extension (.xml).
There was a problem hiding this comment.
Looks good to me.
Linearisation strategy is still a subject of disagreement but if by default the signal is loaded as nua I guess it is the user's responsibility to linearize in the way it pleases him/her (and decide how the linearization error is dealt with).
Cheers
Nico
pietsjoh
force-pushed
the
jobin-yvon
branch
from
fa32a1e
to
f4b5dc3
Compare
pietsjoh
force-pushed
the
jobin-yvon
branch
from
f4b5dc3
to
f1194fc
Compare
Description of the change
Add support for reading the .xml-format from jobin-yvon.
Progress of the PR
upcoming_changesfolder (seeupcoming_changes/README.rst),readthedocsdoc build of this PR (link in github checks)Minimal example of the bug fix or the new feature