compute_UB() should return UB matrix #85
Conversation
hkl/sample.py
Outdated
|
|
||
| for name in ('lattice', 'name', 'U', 'UB', 'ux', 'uy', 'uz', | ||
| 'reflections', ): | ||
| raise ValueError("Invalid unit type") |
There was a problem hiding this comment.
Since we are here, it'd probably make sense to improve this error message, at least by reporting what unit was used, and what are the allowed units.
There was a problem hiding this comment.
in practice, we use the default units always. So I guess the default value for units is "user" so these text strings will match. if the user accidentally entered a float in nm (not Angstrom) for a, b, and c, then it will not be caught unless we start explicitly defining the units. I would say the most people are lazy and will want to do what spec assumes.
I think there more important issues to address (get spec features that are missing) as opposed to robust unit fault finding, but I am fine if @prjemian wants to address unit type now. I just don't think there is much value gained unless we force people to include proper units.
There was a problem hiding this comment.
For this PR, I'll just improve the detail in the exception message (received v. expected).
There is a deeper issue in the libhkl code where the units are actually something engineering (degrees for rotations as an example) but the hklpy code expects default or user. Need to identify a clear example before we could consider resolving how to handle units properly.
There was a problem hiding this comment.
For this PR, I'll just improve the detail in the exception message (received v. expected).
I was asking exactly for that, thanks @prjemian!
hkl/tests/test_sample.py
Outdated
| import pytest | ||
|
|
||
| import gi | ||
| gi.require_version('Hkl', '5.0') |
There was a problem hiding this comment.
Looks like black ignored this file (' -> " did not happen). Should it format this test file too?
There was a problem hiding this comment.
If we black this now, we'll get a merge conflict with another PR. Is it easier to resolve conflicts after code styling is applied? If we don't style the code now, we might forget...
|
Conflicts with main branch. Merged from main rather than rebase due to multiple conflicting commits involving |
fixes #68, previously
sample.compute_UB(r1, r2)returned 1 or 0 (success, fail) which is not so useful. The underlying code will raise an exception if a failure (such as r1 & r2 are parallel). The code had a comment telling what to do. If we're just going to ignore the return result, it might as well be a useful one.@ambarb Agree?