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

[BUG] fix lookup for specialized test classes #189

Merged
merged 24 commits into from
Jan 30, 2024
Merged

Conversation

fkiraly
Copy link
Collaborator

@fkiraly fkiraly commented Jan 29, 2024

This PR fixes lookup for the specialized test classes.

The internal all_objects call was passing classes as object_type to the skpro registry lookup, which it did not recognize - it should be the type string.

As a consequence, the list of classes was coming up empty - instead of containing classes of a given scitype - hence test coverage was reduced since initially when the skpro all_objects utility was upgraded (which is not long ago but long enough for uncovered failures to creep in). The generic test classes TestAllEstimators, TestAllObjects were not impacted.

Depends on fixes for silent test failures:

@fkiraly fkiraly added bug module:tests test framework functionality - only framework, excl specific tests labels Jan 29, 2024
fkiraly added a commit that referenced this pull request Jan 30, 2024
…#192)

This PR fixes API non-compliances in the `sklearn` variance prediction
adapters, uncovered by #189:

* `sklearn` `ARDRegressor` fails with `X: pd.DataFrame` on `predict`
with `return_std=True`
scikit-learn/scikit-learn#28310
* `skpro` `SklearnProbaReg.predict` returned `np.ndarray` even if `y` in
`fit` was `pd.DataFrame`
@fkiraly fkiraly closed this in #191 Jan 30, 2024
fkiraly added a commit that referenced this pull request Jan 30, 2024
…tiles` when only `_predict_var` is implemented (#191)

This PR fixes a unreported bug in `BaseProbaRegressor` where
`_predict_interval` and `_predict_quantiles` would raise an
`NotImplementedError` when only `_predict_var` is implemented - despite
current defaulting logic being sufficient to obtain (normal) quantiles
and symmetric (normal) intervals from `_predict` and `_predict_var`.

This issue is present with the `sklearn` adapter and delegate
descendants, but was not uncovered due to masking by the test bug in
#189.

The failure is present in test logs of the fix
#189, and #189 is used for testing
(by merge into).
@fkiraly
Copy link
Collaborator Author

fkiraly commented Jan 30, 2024

closed by accident?

@fkiraly fkiraly reopened this Jan 30, 2024
fkiraly added a commit that referenced this pull request Jan 30, 2024
…olved (#193)

This PR adds a skip for failures of `CyclicBoosting` and QPD objects on
main, until failures in #189 are resolved.

The PR resolving the failures should remove the skips again.
fkiraly added a commit that referenced this pull request Jan 30, 2024
@fkiraly fkiraly merged commit d3122d8 into main Jan 30, 2024
35 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug module:tests test framework functionality - only framework, excl specific tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant