Write proposal and run numbers to virtual CXI files. - #749
Conversation
daniloefl
left a comment
There was a problem hiding this comment.
Looks great! Feel free ignore all my comments. They are mostly only ideas for alternative implementations, in case you think it is worth going in those directions.
| # the CXI standard, but it allows extra data. | ||
| run_metadata = self.detdata.data.run_metadata() | ||
| if 'proposalNumber' in run_metadata and 'runNumber' in run_metadata: | ||
| f.create_dataset(f'entry_1/proposalNumber', |
There was a problem hiding this comment.
I can see you are just following a previous standard, but is there a reason for entry_1?
There was a problem hiding this comment.
According to CXI format convention, entry_1 should correspond to the experiment.
|
Thank you for your comments, Danilo! The are all very good. We should try to follow CXI format guide: |
|
I also do not have an opinion here. Your usage of |
|
You may want to consider whether attributes are already a concept for metadata in CXI or not, and whether you want this EuXFEL-specific information to be visible or rather hidden (note I do not have answers or opinions on this out of ignorance). |
| # Store detector name as an attribute | ||
| dgrp.attrs['name'] = self.detdata.detector_name |
There was a problem hiding this comment.
I'm adding name of the detector as 'detector_1' attribute.
|
This broadly LGTM. One more naming bikeshed question, though: do we want to make it explicit that the proposal number is an EuXFEL proposal number? EuXFEL proposal number 12345 is unique, and you have an immediate hint where to find more context. |
Although proposal and run numbers are not part of the CXI format, I find them useful additional information.