reflectivity / orsopy

http://www.reflectometry.org/orsopy/
MIT License
0 stars 8 forks source link

Specification of Column units attribute values? #125

Open bmaranville opened 9 months ago

bmaranville commented 9 months ago

There is currently a test in orsopy.fileio.tests.test_schema that is expected to fail if the units for the first column are set to to the string: "test_fail_unit"

This is a problem, since in principle the Column schema needs to be applied to not just the first two columns ["Qz", "R"], but to any additional columns as well (after the first 4 columns, such as wavelength, incident_angle, etc.) which will not have the units specified manually in the current schema: {"enum": ["1/angstrom", "1/nm", "1", "1/s", None]}

We can address this in a couple ways:

  1. create a separate schema for additional columns, in which unit is any string
  2. change the Column schema so that unit is any string (which is the schema already defined in the Python code - the JSON schema is being manually modified to make it more restrictive in this spot)
andyfaff commented 9 months ago

Is it possible to have the enumerated units for the first four columns, and any other string for additional columns, without creating a separate schema for those extra columns?

Will solution 2 permit any units for any column? I guess I don't mind this so much. If the default units aren't one of the enumerated ones I'd just go for the default of 1/angstrom.

Ultimately I prefer whatever solution is easiest to maintain.

aglavic commented 9 months ago

I support solution 2 with a custom code in _read_header_data that check this additional constraint.

Lookin at it I realized that there is actually no user function to trigger the schema validation. We should consider adding it to either the Orso class directly (for check in safe and load scenarios) or to load_orso as keyword argument.