-
Notifications
You must be signed in to change notification settings - Fork 2
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
First multi reader version #10
Conversation
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.
Most of it looks fine already, I added some relatively minor comments.
What I don't understand is why you define a second reader option raman_multi
instead of having a single raman
reader that can read the witec data and also whatever data the raman
. Is that something you want to do in the future or are these supposed independent going forward?
@lukaspie Yea, I want to remove this, but not 100% sure if I break something, if I just replaced it. Maybe Ill do it in the end of this PR then. Thanks for all your remarks, Ill go through this. |
So, now also a CI/CD is implemented for the multiformat reader. Should be fine from my side - if this is approved, we could merge this in. |
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.
lgtm
This implements the addiitonal reader option "raman_multi". (The old PR9 will be deleted).
This is a reader for Raman data, which also is valid for the eln_data.yaml file + measurement file from the witec alpha Raman spectrometer - BUT it is now based on the Multiformat reader.
This will enable future compatibility with various Raman measurement devices.
The folder and file structure has later to be reorganized, if the next reader is implemented.
Some remaining warnings are present:
https://github.com/FAIRmat-NFDI/pynxtools-raman/compare/switch_to_multi_format_reader2?expand=1#diff-2847ccb27aeae8b5edbbd79029b17be1ba2042830d54cb0698a2e351cce91022R235-R242