NXcxi_ptycho contributed definition #628
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Two errors: Remove the |
|
You can repeat this test yourself by running |
This doesn't test the contributed definitions. Do I have to change something? |
|
Hmmm... It should. When run in travis-ci (see above report), contributed definitions were run, otherwise there would not have been a report. |
|
Thanks @aaron-parsons |
|
@prjemian It does now. I was being silly and hadn't updated my fork in a while so it didn't include changes to this test. It does now, and everything passes so the merge should be straight forward. |
|
Confirmation that contributed definitions are tested from command line: testing of this specific PRsame, but includes the new classes |
|
Changes to |
|
Ok I've corrected that now. |
|
Interesting that this was not caught by the previous testing. The Sphinx documentation caught this error. There is no NeXus base class called This means to change only two places in the NXDL file:
I'm not certain how best to resolve lines 224-226 of the NXDL. Perhaps replace: with |
|
Thanks for reviewing this. These both come about because we are trying to make this compatable with the CXI format. Technically these two entries don't need to be NeXus fields at all and are only there to ensure the compatibility on the CXI side. I'd rather not make line 202 an NXdata, because it isn't nexus formatted, and can't be. This would break things. I can see two options to get around this, or I'm happy to take another course if you can see one:
What do you think? Aaron |
|
I've converted them to NXnote now so let me know if that isn't a good solution. @markbasham do you have any thoughts on this? |
|
Ah, good. In the first case (line 202), the best base class to use is NXcollection which is used to contain non-NeXus content. You've described this exactly. |
|
Ok. I've switched both out for NXcollection after looking at the description for this base class. One thing that concerns me slightly is where it says: "For NeXus validation, NXcollection will always generate a warning since it is always an optional group.". It's not really an optional group, because we need it for the CXI validation. So if we really want this to be compatible both it should be a required group, but one that isn't validated by NeXus since it's not NeXus content. Does it sound ok? |
|
@markbasham Have renamed this as requested. |
|
@aaron-parsons sorry other way round NXcxi_ptycho then i think we are all good to go. |
|
@markbasham Right you are. All done. |
|
change the name of this PR to new name of NXDL class |
prjemian
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
This contains a applications definition for NXptycho for ptychography experiments. It's come about after talking to the CXI (another file format for coherent imaging experiments) community and merges the two so that NXptycho is CXI compatible. We are starting to use this at Diamond now on multiple beamlines.
I'll also be adding a PR to nexusformat/exampledata with some example files for you to look through.
Let me know if something wasn't clear in the xml and I can change it, I wasn't sure at which level I needed to specify things, but hopefully the doc makes it clear.
I needed to add an extra field to NXbeam so thats included here too. It's just information about the extent of the beam, which is useful for ptychography.
Aaron
@markbasham