Skip to content

Conversation

@rowlesmr
Copy link
Collaborator

@rowlesmr rowlesmr commented Jun 4, 2023

Adding examples of use at the category level.

Not updating update dates yet.

Categories:

  • PD_AMORPHOUS
  • PD_BACKGROUND
  • PD_BLOCK
    • example is in _pd_block.id, but it's the only member.
  • PD_CALC_COMPONENT
  • PD_CALC_OVERALL
  • PD_CALIB_D_TO_TOF
  • PD_CALIB_DETECTED_INTENSITY
  • PD_CALIB_INCIDENT_INTENSITY
  • PD_CALIB_WAVELENGTH
  • PD_CALIB_XCOORD
  • PD_CALIBRATION
  • PD_CHAR
  • PD_DATA (containing PD_CALC, PD_MEAS, PD_PROC)
  • PD_DIFFRACTOGRAM
    • example is in _pd_diffractogram.id, but there're only two members.
  • PD_INSTR
  • PD_INSTR_DETECTOR
  • PD_MEAS_OVERALL
  • PD_PEAK
  • PD_PEAK_OVERALL
  • PD_PHASE
    • examples are in the individual members
  • PD_PHASE_MASS
  • PD_PREF_ORIENT_MARCH_DOLLASE
  • PD_PREF_ORIENT_SPHERICAL_HARMONICS
  • PD_PREP
  • PD_PROC_LS
  • PD_PROC_OVERALL
  • PD_QPA_CALIB_FACTOR
  • PD_QPA_EXTERNAL_STD
  • PD_QPA_INTENSITY_FACTOR
  • PD_QPA_INTERNAL_STD
  • PD_QPA_OVERALL
  • PD_SPEC

Not (currently) going to, as other categories are taking over, or have taken over:

  • PD_BLOCK_DIFFRACTOGRAM
  • PD_CALIB
  • PD_CALIB_OFFSET
  • PD_CALIB_STD
  • PD_MEAS_INFO_AUTHOR
  • PD_PHASE_BLOCK
  • PD_PREF_ORIENT
  • PD_PROC_INFO_AUTHOR

@rowlesmr
Copy link
Collaborator Author

I don't think that _pd_calib_detected_intensity.id is a necessary category key, but also:

Apart from that I think including an identifier data name in the key is fine, e.g. you just keep calibrating the instrument every week and assign an identifier to each calibration #92 (comment)

@rowlesmr
Copy link
Collaborator Author

rowlesmr commented Jun 26, 2023

I don't think that _pd_calib_detected_intensity.id is a necessary category key, but also:

Apart from that I think including an identifier data name in the key is fine, e.g. you just keep calibrating the instrument every week and assign an identifier to each calibration #92 (comment)

I think we should keep _pd_calib_detected_intensity.id (due to being able to record multiple calibrations on the same instrument over time*), but I am now unsure that _pd_calib_detected_intensity.diffractogram_id is doing it's job.

If I have

data_unknown_diffractogram
_pd_diffractogram.id UNKNOWN_SPECIMEN
    
loop_
_pd_calib_detected_intensity.id
_pd_calib_detected_intensity.detector_id
_pd_calib_detected_intensity.detector_response
_pd_calib_detected_intensity.special_details
1   1_4913c6ed   1       'Scanned through direct beam.'
1   2_4913c6ed   0.973   'Scanned through direct beam.'
1   3_4913c6ed   0.997   'Scanned through direct beam.'
1   4_4913c6ed   1.039   'Scanned through direct beam.'
#...

then _pd_calib_detected_intensity.diffractogram_id gets the value 'UNKNOWN_SPECIMEN', when the intention is to record the diffractogram which was used to make the calibration, if such a diffractogram exists (which doesn't, in this case).

I think this ties in to #163.

This whole argument also applies to _pd_calib_incident_intensity.diffractogram_id

* I need to add a key to PD_CALIB_INCIDENT_INTENSITY.

@rowlesmr
Copy link
Collaborator Author

These examples need to be checked in light of changes over time in the rest of the dictionary.

Copy link
Collaborator

@vaitkus vaitkus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the work, I noted a few minor discrepancies.

However, do you think it would be possible to split the PR into several more manageable parts? There are several example that could be merged right away while others may need some more time to digest.

@rowlesmr
Copy link
Collaborator Author

However, do you think it would be possible to split the PR into several more manageable parts?

Boo. I was coming to the same conclusion...

I'll see if I can get it done tonight.

@rowlesmr
Copy link
Collaborator Author

rowlesmr commented Jun 19, 2025

PD_CALIB_WAVELENGTH needs to be fixed before updating the examples has been deleted
PD_CALIB_XCOORD needs to be rejigged before updating the examples
PD_QPA_EXTERNAL_STD needs to be updated before doing the examples PD_DIFFRACTOGRAM now has link to _pd_instr.id, so no update needed.

@vaitkus
Copy link
Collaborator

vaitkus commented Jun 19, 2025

Thanks! This will make the reviewing and merging much simpler. Let's leave this PR open so once we merge all we others, we could resync just to see if we missed anything.

@vaitkus
Copy link
Collaborator

vaitkus commented Jul 9, 2025

I think that all of the PR that splintered of this one have now been merged. @rowlesmr would probably be the best one to decide how to continue with this PR (e.g. close it outright or try to sync it with the main branch to see if nothing important got accidentally left out). As this branch will not get merged, fully resolving all of the sync conflict might not be the best use of our time, but starting a merge and quickly going through the diff list might still provide some insights.

@rowlesmr rowlesmr closed this Jul 11, 2025
@rowlesmr rowlesmr deleted the add-examples-2 branch July 11, 2025 13:37
@rowlesmr rowlesmr restored the add-examples-2 branch July 11, 2025 13:44
@rowlesmr rowlesmr reopened this Jul 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants