Fixed reordering of markers following PyoMarkers #70

EveCharbie · 2025-07-10T11:17:24Z

I don't understand why it changed anything, but now it seems to work better...

This change is

EveCharbie · 2025-07-11T08:58:01Z

@Ipuch verry small modifs to review :)

Ipuch

Very small but still edgy, some comments to guarantee the code doesn't drift too much into something not maintainable for me.

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @EveCharbie)

pyorerun/utils/markers_utils.py line 38 at r1 (raw file):

        for i_marker, marker in enumerate(model.marker_names):
            marker_index = tracked_marker_names.index(marker)
            reordered_markers[:, i_marker, :] = tracked_markers.to_numpy()[:, marker_index, :]

I want a function for this piece of code and a test:

def sort_markers_to_a_predefined_order(markers: np.ndarray, initial_name_list, wanted_name_list):
"""
try to have a precise doc
"""
reordered_markers = np.zeros_like(tracked_markers.to_numpy())             
for i_marker, wanted_name_list in enumerate(wanted_name_list):
                       name_index = initial_name_list.index(marker_name_wanted)
                       reordered_markers[:, i_marker, :] = markers[:, name_index, :]
            return reordered_markers

Code quote:

        reordered_markers = np.zeros_like(tracked_markers.to_numpy())
        for i_marker, marker in enumerate(model.marker_names):
            marker_index = tracked_marker_names.index(marker)
            reordered_markers[:, i_marker, :] = tracked_markers.to_numpy()[:, marker_index, :]

tests/test_not_all_markers_in_models_and_tracked_makers.py line 57 at r1 (raw file):

def test_tracked_makers_not_in_order():

typo : test_tracked_markers_not_in_order

pyorerun/phase_rerun.py line 125 at r1 (raw file):

        """Add the tracked markers to the phase."""

        tracked_markers = check_and_adjust_markers(model, tracked_markers)

It's okay to me to remove it, but It was indeed cleaner to keep it inside. I don't understand why this is need, we need more tests. We must have something wrong somewhere. I dislike it?

pyorerun/rrc3d.py line 70 at r1 (raw file):

    phase_rerun.add_xp_markers(filename, pyomarkers)

    lowest_corner = 0.0

remove

pyorerun/rrc3d.py line 73 at r1 (raw file):

    if show_force_plates:
        force_plates_corners = get_force_plates(c3d_file, units=units)
        lowest_corner = get_lowest_corner(c3d_file, units=units)

Move this under 'if show_floor'

Code quote:

lowest_corner = get_lowest_corner(c3d_file, units=units)

pyorerun/rrc3d.py line 102 at r1 (raw file):

    if show_floor:
        square_width = max_xy_coordinate_span_by_markers(pyomarkers)

add :
lowest_corner = 0 if not show_force_plates else lowest_corner

pyorerun/phase_rerun.py line 91 at r1 (raw file):

        if isinstance(tracked_markers, np.ndarray):
            tracked_markers = PyoMarkers(tracked_markers, channels=model.marker_names)

Do we have a problem here?
I've explicitly asked the user to fill in PyoMarkers in the past because I wanted him to order the markers correctly. Letting the user give a np.array is a bad idea; this was my initial thought.

Code quote:

        if isinstance(tracked_markers, np.ndarray):
            tracked_markers = PyoMarkers(tracked_markers, channels=model.marker_names)

pyorerun/phase_rerun.py line 100 at r1 (raw file):

        if tracked_markers is not None:
            tracked_markers = check_and_adjust_markers(model, tracked_markers)

I feel a unit test of this function would strengthen my trust about this PR.

Code quote:

check_and_adjust_markers(model, tracked_markers)

EveCharbie

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @Ipuch)

pyorerun/phase_rerun.py line 91 at r1 (raw file):