Skip to content

Conversation

@dexter2206
Copy link
Contributor

Description

We need to preserve chronology of children when computing highwater in Bartiq. However, the chronology is lost when we perform validation of RoutineV1, because we sort children by name. This PR just removes the sorting.

Note that all other functionalities we have here and in Bartiq don't rely on chronology, so this change should be non-breaking.

Please verify that you have completed the following steps

  • I have self-reviewed my code.
  • I have included test cases validating introduced feature/fix.
  • I have updated documentation.

@dexter2206 dexter2206 requested a review from mstechly February 17, 2025 16:48
@cla-bot cla-bot bot added the cla-signed label Feb 17, 2025


def test_chronology_of_children_is_preserved_when_constructing_model_instances():
# We used to sort children by name before we decided chronology is important. This test ensures that
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you find the right place to mention it in the docs as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent suggestion, I mentioned that we don't sort children in the docs, see 69e5b9d

@dexter2206 dexter2206 merged commit 3869933 into main Feb 17, 2025
6 checks passed
@dexter2206 dexter2206 deleted the kj/dont-sort-children branch February 17, 2025 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants