NXopt workshop updates - Githubissues

FAIRmat-NFDI / nexus_definitions

Definitions of the NeXus Standard File Structure and Contents

https://manual.nexusformat.org/

Other

5 stars 8 forks source link

NXopt workshop updates #242

Closed RonHildebrandt closed 1 day ago

RonHildebrandt commented 2 weeks ago

This PR contains the updates of NXopt (now renamed to NXoptical_spectroscopy) from the optical spectroscopy workshop in Berlin. Most changes are located in the NXoptical_spectroscopy definition.

Major adjustments: NXoptical_spectroscopy

Included reference frame description,
updated exists/recommended requirements,
restructruring the order in the instrument level.

Minor adjustments:

Adjustments for NXopt_window, NXlens_opt, NXwaveplate were things, which were originally located in NXopt but are of general relevance.
NXellipsometry had some minor adjustments for the the "exists:" fields.
NXraman contained only the NXopt renaming and minor "exists:" adjustments.
For the creation of the manual, the ellipsometry-structure.rst was adjusted, so it contains as well the NXopt renaming.

With this adjustmenets, NXoptical_spectroscopy should be finalized.

tomio13 commented 1 week ago

I am looking at the NXoptical_spectroscopy. On line 182, why is the beam_TYPE half capitalized?

Line 201: incident_wavelength: From the usage of spread in the consecutive lines, I would assume this is the central wavelength of a spectral range. Please specify this in the doc string of this field.

In Line 207: what does extent stand for? Spatial size? Temporal duration?

In line 229 the docstring is incomplete.

In line 290: "Specify respective hardware which was used to for the detector. "... should be probable 'Specify respective hardware which was used for the detector. '

... for now

RonHildebrandt commented 4 days ago

@tomio13 Line 201 and Line 207: I did not specify this as, I wanted to refer to NIAC definitions: https://manual.nexusformat.org/classes/base_classes/NXbeam.html In principle, these should be transfered as well to the html site (https://fairmat-nfdi.github.io/nexus_definitions/NXopt_workshop_updates/classes/contributed_definitions/NXoptical_spectroscopy.html#nxoptical-spectroscopy-entry-instrument-lens-opt-type-field). But this is not the case right now.

Line 229: Finished the docstring.

Line 290: Done.

RonHildebrandt commented 2 days ago

@sanbrock I adressed everything. Hope everything is fine now to merge this in.

domna commented 1 day ago

@RonHildebrandt Could you change the definition/@url to definition/@URL? Then it would match the concept NXentry/definition/@URL and would be in alignment with https://github.com/FAIRmat-NFDI/pynxtools/pull/361

RonHildebrandt commented 1 day ago

@domna added the requested changes