348 replaced point 3d and point 2d to support arithmetic, using numpy arrays#623
Conversation
…rtemis into 348_replace_point_3d_to_support_arithmetic
Codecov Report
@@ Coverage Diff @@
## main #623 +/- ##
==========================================
- Coverage 91.59% 91.56% -0.04%
==========================================
Files 35 34 -1
Lines 1665 1659 -6
==========================================
- Hits 1525 1519 -6
Misses 140 140
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
olliesilvester
changed the title
d-perl
left a comment
d-perl
left a comment
There was a problem hiding this comment.
This is a good start, but I think we can simplify it more, and probably get rid of all of utils.py in the end.
If we need to check the length at some point, we could do something like
assert 2 <= len(point) <= 3but for the parameters, they are validated by the json schema already so we are always sure they are length 3
| 100 | ||
| ) | ||
| expected_centre_motor_coords = ( | ||
| dummy_params.experiment_params.grid_position_to_motor_position( |
There was a problem hiding this comment.
so part of the reason for this whole refactor is so that we could do something tidier like
expected_centre_grid_coords - [0.5, 0.5, 0.5]| mock_zocalo_functions(callbacks) | ||
| callbacks.ispyb_handler.ispyb_ids = (0, 0, 100) | ||
| expected_centre_grid_coords = Point3D(1, 2, 3) | ||
| expected_centre_grid_coords = create_point(1, 2, 3) |
There was a problem hiding this comment.
so this would just be expected_centre_grid_coords = ndarray([1,2,3])
| assert found_centre == expected_centre_motor_coords | ||
| np.testing.assert_array_equal(found_centre, expected_centre_motor_coords) | ||
|
|
||
| expected_bbox_size = list(map(operator.sub, [8, 8, 7], [2, 2, 2])) |
There was a problem hiding this comment.
while we're numpyifying stuff, this could be ... = ndarray([8, 8, 7]) - ndarray([2, 2, 2]), and in the real code where this happens
| # gets stored as 2x2D coords - (x, y) and (x, z). Values in pixels | ||
| "upper_left": Point3D(x=0, y=0, z=0), | ||
| "position": Point3D(x=0, y=0, z=0), | ||
| "upper_left": create_point(0, 0, 0), |
There was a problem hiding this comment.
this would just be something like "upper_left": [0,0,0]
d-perl
left a comment
d-perl
left a comment
There was a problem hiding this comment.
looks great, sorry I left this so long that there are all these merge conflicts
2 participants
Fixes #348
Link to dodal PR: #37
To test: