Add sample to source distance to Beam

[toastisme]

This adds sample_to_source_distance_in_m to Beam. This is required for time-of-flight experiments (to actually calculate the time-of-flight), but I thought it could potentially be useful in other experiments (e.g. an alternative beam divergence model), so have added it to Beam rather than a derived class.

This also adds some additional Beam similarity checks that were missing.

This branches off of #621.

[codecov]

Codecov Report

Merging #622 (194746b) into main (3bc6e67) will decrease coverage by 0.99%.
Report is 6 commits behind head on main.
The diff coverage is 0.73%.

❗ Current head 194746b differs from pull request most recent head febaa0c. Consider uploading reports for the commit febaa0c to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #622      +/-   ##
==========================================
- Coverage   39.10%   38.12%   -0.99%     
==========================================
  Files         181      182       +1     
  Lines       15864    16270     +406     
  Branches     3066     3161      +95     
==========================================
- Hits         6204     6203       -1     
- Misses       9074     9481     +407     
  Partials      586      586              

[graeme-winter]

@toastisme not a criticism, but sample_to_source_distance_in_m feels awkward and does not "fit" i.e. I don't think we usually include the unit unless there is an alternative API (e.g. pixel / mm) and we also (again, I think) use mm everywhere not m

I make the comment without an implication of what is or is not correct

[rjgildea]

Would it make sense to start returning a pint.Quantity , at least on the Python side, in places such as this, then it is unambiguous in terms of unit, without needing the unit hardcoding in the method/property name?

[dagewa]

I suggest changing name to sample_to_source_distance in this PR, expressed in mm for consistency with other lab distance quantities, then set up an issue to introduce pint.Quantity in all relevant areas separately.

[ndevenish]

Okay, fixing this merge conflict was significantly more involved than I had expected. The fact that the Polybeam PR was merged (and this branch was on top of an earlier, before-rename, version of that branch) and that the Probe PR also touches many of the same places as this, caused git to get extremely confused. (I think a "clever" approach I tried early also turned out to be "the most difficult path").

I think this is okay, but not confident enough to merge. @toastisme, could you look over and make sure this updated branch makes sense?

[toastisme]

Okay, fixing this merge conflict was significantly more involved than I had expected. The fact that the Polybeam PR was merged (and this branch was on top of an earlier, before-rename, version of that branch) and that the Probe PR also touches many of the same places as this, caused git to get extremely confused. (I think a "clever" approach I tried early also turned out to be "the most difficult path").

I think this is okay, but not confident enough to merge. @toastisme, could you look over and make sure this updated branch makes sense?

Ah what a mess. Thanks for sorting this out. Looks good to me.