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

Experimental - DO NOT MERGE: skbase refactor - part 3: test framework #4283

Open
wants to merge 85 commits into
base: main
Choose a base branch
from

Conversation

fkiraly
Copy link
Collaborator

@fkiraly fkiraly commented Mar 4, 2023

Experimental refactor branch.

Not for merge until mature, tests are in place, and based on current skbase.

Moves BaseObject and core base functionality to the skbase package.

Part 3, relies on:

Migrated:

  • BaseFixtureGenerator, QuickTester, and TestAllObjects, uses skbase.testing under the hood.
  • from part 2 - all_estimators - uses skbase.all_objects under the hood now.
  • from part 1 - BaseObject. The sktime BaseObject is a child of skbase BaseObject with some added features that are currently sktime specific.

This should be the last step of the refactor/migration.

@fkiraly fkiraly added maintenance Continuous integration, unit testing & package distribution refactor Restructuring without changing its external behavior. Neither fixing a bug nor adding a feature. labels Mar 4, 2023
@fkiraly fkiraly changed the title Experimental - DO NOT MERGE: BaseObject refactor - part 3: test framework Experimental - DO NOT MERGE: skbase refactor - part 3: test framework Mar 4, 2023
@fkiraly
Copy link
Collaborator Author

fkiraly commented Mar 5, 2023

@RNKuhns, I think I've refactored the sktime test framework correctly, but there's now a highly mysterious bug appearing.

The tests seem to be run as expected, but one of the tests - multiprocessing identity (check results are the same with and without multiprocessing) - fails with an odd error, where out of 10 entries in the result a few (four) are by a constant of 1 off.

I cannot explain this with missing excluded tests etc, especially since it's coming from TestAllEstimators rather than TestAllObjects.

@fkiraly fkiraly mentioned this pull request Feb 21, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Continuous integration, unit testing & package distribution refactor Restructuring without changing its external behavior. Neither fixing a bug nor adding a feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant