Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework Python tests to use pytest, enforce coverage #335

Merged
merged 4 commits into from
Feb 18, 2025

Conversation

kroenlein
Copy link
Contributor

This PR is primarily focused on improving tests on lolopy. It should include no changes to functionality.

Note that coverage is skipped on recursive calls to _make_learners when there are user-defined learners present. It was not clear how to exercise those routes.

  • Migrate tests to use pytest, allowing for monkeypatching, and numpy.testing where appropriate
  • Rename version.py to version.py, per standard as not a user-facing module
  • Combine deploy actions into one script
  • Update testing to require 100 % coverage for Python files
  • Change __get_state__ in BaseLoloLearner to always copy the state dictionary, as the result returned from latest sklearn uses a tied dictionary, so deleting gateway corrupted the object. Also change the del to a pop so, removing the need for a try / except block.
  • Add __init__ to BaseLoloClassifier so that n_classes is not initialized outside of an __init__ method
  • Add testing for pickling and migrate testing for json serialization to use a temporary directory
  • Migrate file handling from os to pathlib

Copy link
Contributor

@mVenetos97 mVenetos97 left a comment

Choose a reason for hiding this comment

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

LGTM

@kroenlein kroenlein merged commit 8b539a4 into main Feb 18, 2025
11 checks passed
@kroenlein kroenlein deleted the maintain/boost-test-coverage branch February 18, 2025 15:15
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.

2 participants