- 
                Notifications
    You must be signed in to change notification settings 
- Fork 23
apply data_range to all columns, including T, dT, L, dL #331
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
base: master
Are you sure you want to change the base?
Changes from all commits
8b58e8a
              8417602
              0c82c52
              2605545
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -87,26 +87,46 @@ def get_key(orso_name, refl1d_name, refl1d_resolution_name): | |
| (i for i, c in enumerate(columns) if getattr(c, "physical_quantity", None) == orso_name), | ||
| None, | ||
| ) | ||
| resolution_index = None | ||
| if column_index is not None: | ||
| # NOTE: this is based on column being second index (under debate in ORSO) | ||
| header_out[refl1d_name] = data[:, column_index] | ||
| cname = columns[column_index].name | ||
| resolution_index = next( | ||
| (i for i, c in enumerate(columns) if getattr(c, "error_of", None) == cname), | ||
| None, | ||
| resolution_index, resolution_column = next( | ||
| ((i, c) for i, c in enumerate(columns) if getattr(c, "error_of", None) == cname), | ||
| (None, None), | ||
| ) | ||
| if resolution_index is not None: | ||
| header_out[refl1d_resolution_name] = data[:, resolution_index] | ||
| else: | ||
| v = getattr(settings, orso_name, None) | ||
| if hasattr(v, "magnitude"): | ||
| header_out[refl1d_name] = v.magnitude | ||
| if hasattr(v, "error"): | ||
| header_out[refl1d_resolution_name] = v.error.error_value | ||
| if resolution_column.value_is == "FWHM": | ||
| header_out[refl1d_resolution_name] = FWHM2sigma(header_out[refl1d_resolution_name]) | ||
|  | ||
| # Fall back to instrument_settings if no column found | ||
| v = getattr(settings, orso_name, None) | ||
| if hasattr(v, "magnitude") and column_index is None: | ||
| # only set if not already set from column | ||
| header_out[refl1d_name] = v.magnitude | ||
| if hasattr(v, "error") and resolution_index is None: | ||
| header_out[refl1d_resolution_name] = v.error.error_value | ||
| if v.error.value_is == "FWHM": | ||
| header_out[refl1d_resolution_name] = float(FWHM2sigma(header_out[refl1d_resolution_name])) | ||
|  | ||
| get_key("incident_angle", "angle", "angular_resolution") | ||
| get_key("wavelength", "wavelength", "wavelength_resolution") | ||
|  | ||
| # convert error columns from FWHM to sigma if they are present | ||
| # and need conversion | ||
| if len(columns) >= 3: | ||
| # third column is always uncertainty on reflectivity | ||
| dR_col = columns[2] | ||
| if dR_col.value_is == "FWHM": | ||
| data[:, 2] = FWHM2sigma(data[:, 2]) | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This modifies the entry in place as returned by the orso/nexus reader. That's probably okay, but since you are copying the data anyway on line 130, you may as well move the copy to line 62 so that you are operating on your own copy of the data. You might also force the data type to be float64 when you create the copy. I'm a little concerned that ΔR is potentially being scaled by FWHM2sigma. Is there any risk that the ΔR column is mislabeled as FWHM, e.g., because it is defaulting to FWHM when the value isn't specified? This would be happening in the orso/nexus readers. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, there's a risk that the dR is not properly tagged as  At least, in the implementation here, if the ORSO file is correctly described, it will correctly load, even if  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess we could separate the parse step from the step that fills in the Probe classes, and allow users to edit parts of the ORSO classes before loading them... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the column is mislabeled in the stored file that's on the facility. The user can correct this manually after loading the file. My concern was whether the orso or nexus loaders default the ΔR column to FWHM if it is not otherwise labelled in the file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, the  | ||
| if len(columns) >= 4: | ||
| # fourth column is always uncertainty on Q | ||
| dQ_col = columns[3] | ||
| if dQ_col.value_is == "FWHM": | ||
| data[:, 3] = FWHM2sigma(data[:, 3]) | ||
|  | ||
| entries_out.append((header_out, np.array(data).T)) | ||
| return entries_out | ||
|  | ||
|  | @@ -232,6 +252,8 @@ def load4( | |
|  | ||
| if filename.endswith(".ort") or filename.endswith(".orb"): | ||
| entries = parse_orso(filename) | ||
| FWHM = False # ORSO files specify sigma vs. FWHM and will be converted to sigma | ||
| # TODO: ORSO also specifies resolution type (normal vs. uniform vs. triangular etc.) | ||
| else: | ||
| json_header_encoding = True # for .refl files, header values are json-encoded | ||
| entries = parse_multi(filename, keysep=keysep, sep=sep, comment=comment) | ||
|  | @@ -345,7 +367,7 @@ def fetch_key(key, override): | |
| return override | ||
| elif key in header: | ||
| v = decoder(header[key]) | ||
| return np.array(v)[index] if isinstance(v, list) else v | ||
| return np.array(v)[index] if hasattr(v, "__getitem__") else v | ||
| else: | ||
| return None | ||
|  | ||
|  | ||
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.
get_key() does not need to be defined in the for loop. It picks up the local scope regardless of where things are defined, so long as it is defined before it is used.