Skip to content

Conversation

@alessandrofelder
Copy link
Member

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

What does this PR do?

Makes check_training_data_exists more strict, by requiring both non-empty training data layers to return True.
Also adds tests and docstring for this function.

References

Closes #527

How has this PR been tested?

I have run new tests locally without the xfail and they pass, only after the changes made here to the source code, but not before.

Is this a breaking change?

Yes - users can't get away with one empty or non-existent training data layer anymore.

Does this PR require an update to the documentation?

Our tutorial still works the same, so no change needed there. This change enforces good practice with users.
Have added a docstring to a function that didn't have one.

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality (unit & integration)
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit

@alessandrofelder alessandrofelder marked this pull request as ready for review April 30, 2025 16:47
@alessandrofelder alessandrofelder requested a review from a team April 30, 2025 16:47
Copy link
Member

@IgorTatarnikov IgorTatarnikov left a comment

Choose a reason for hiding this comment

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

Looks good! I found the toast at the bottom of the screen hard to catch, since this interrupts the workflow should it use display_info from qt-niu to show a pop up? I don't think it's required so I'll approve and let you decide!

@alessandrofelder
Copy link
Member Author

As discussed internally, we should loosen the strictness on the layer emptiness as per @adamltyson's suggestion, to allow users to iteratively add points to a single training data layer, but probably warn users that their final retraining data should be a balanced mix of cells/non-cells

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve curation.py::check_training_data_exists()

3 participants