test function for get_angle_index #191
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #191 +/- ##
==========================================
- Coverage 96.53% 94.19% -2.35%
==========================================
Files 8 8
Lines 289 310 +21
==========================================
+ Hits 279 292 +13
- Misses 10 18 +8
|
sbillinge
left a comment
sbillinge
left a comment
There was a problem hiding this comment.
We may want to generalize this to get the index of any x-array, not just angles? Also, I could think of more edge cases so we probably want to use purest.parametrize
| the index of the value in the array | ||
| """ | ||
| if self.on_xtype(xtype) is None: | ||
| raise ValueError(_xtype_wmsg(xtype)) |
There was a problem hiding this comment.
I think I can remove this if I let on_xtype() to raise an error for invalid xtypes. Will do that in the other PR.
|
@sbillinge ready for review! |
| print(dict2) | ||
| if not dict1.keys() == dict2.keys(): | ||
| equal = False | ||
| for key in dict1: |
There was a problem hiding this comment.
is this an error or a warning?
There was a problem hiding this comment.
Should be an error. I've edited the error message.
| else: | ||
| assert val1 == val2, f"Values for key '{key}' differ: {val1} != {val2}" | ||
|
|
||
|
|
There was a problem hiding this comment.
we need more cases. Here everything is integers and there is a match. What do we want to happen if the value lies between two other values? Return nearest and a warning? What if it lies outside the range of values? Return nearest and a warning? What if it is really far away? Have a threshold after which we raise and error?
There was a problem hiding this comment.
@sbillinge I addressed this in the new commit. please review.
| xtype = self.input_xtype | ||
| if self.on_xtype(xtype) is None or len(self.on_xtype(xtype)[0]) == 0: | ||
| raise ValueError( | ||
| f"The '{xtype}' array is empty. " "Please ensure it is initialized and the correct xtype is used." |
There was a problem hiding this comment.
The current on_xtype function does not raise an error when the specified xtype is invalid. I was thinking about letting that function raise an error in those cases, but it looks like we can handle invalid xtype together with empty array.
| i = (np.abs(array - value)).argmin() | ||
| nearest_value = np.abs(array[i] - value) | ||
| distance = min(np.abs(value - array.min()), np.abs(value - array.max())) | ||
| threshold = 0.5 * (array.max() - array.min()) |
There was a problem hiding this comment.
I am not super sure about the threshold here. I'm thinking about a dynamic threshold within the reasonable range of each array, but this here can cause a bit of problem if the array is sparse.
| distance = min(np.abs(value - array.min()), np.abs(value - array.max())) | ||
| threshold = 0.5 * (array.max() - array.min()) | ||
|
|
||
| if nearest_value != 0 and (array.min() <= value <= array.max() or distance <= threshold): |
There was a problem hiding this comment.
I am rethinking all this. I wonder if we should just return i and be done with it. Raise an error if the array is empty as you do, but then just return i. what do you think? Then it is up to the user to make sure the result makes sense.
|
this needs a news too. |
2 participants
issue #186: test function for
get_angle_index@sbillinge ready for review