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.

Sign up for GitHub

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

Jump to bottom

[ENH] in all_objects changing filter_tags by str and "list of str" to mean {val: True} rather than val in keys() #326

Closed
fkiraly opened this issue May 11, 2024 · 0 comments · Fixed by #329
Closed

[ENH] in all_objects changing filter_tags by str and "list of str" to mean {val: True} rather than val in keys() #326

fkiraly opened this issue May 11, 2024 · 0 comments · Fixed by #329
Labels
enhancement Adding new functionality implementing framework Implementing core skbase framework

Comments

@fkiraly
Copy link
Contributor

fkiraly commented May 11, 2024

Currently, passing str or list of str to all_objects filter_tags argument will subset the estimators by the estimator having the tag - vs not having it.

This is counterintuitive, as - semantically - users will expect the subsetting by whether the tag is set to True, e.g., if they search for capability:missing_values, they want estimators that can handle missing values.

Unexpectedly, the estimators returned are all classes of types where "missing value handling" is a valid tag, not all estimators that can handle missing values, because it will also retrieve all cases where the tag is set to False.

I think this is such a severe violation of user expectation that we should change it.

Given that it is in a central utility, there should be a deprecation cycle accompanying it.
That is, for a release cycle, users passing str or list of str will be warned that the behaviour will change, with a message of how to keep current behaviour. We should probably also add callables x -> bool as options for fields or similar.

FYI @weenerplasticsgroup.
@fkiraly fkiraly added implementing framework Implementing core skbase framework enhancement Adding new functionality labels May 11, 2024
@fkiraly fkiraly changed the title [ENH] in all_objects changing filter_tags by str and "list of str" to mean {val: True} rather than val in `keys() [ENH] in all_objects changing filter_tags by str and "list of str" to mean {val: True} rather than val in keys() May 11, 2024
@fkiraly fkiraly added bug Something isn't working enhancement Adding new functionality and removed enhancement Adding new functionality bug Something isn't working labels May 14, 2024
@fkiraly fkiraly mentioned this issue May 14, 2024
Merged
@fkiraly fkiraly changed the title [ENH] in all_objects changing filter_tags by str and "list of str" to mean {val: True} rather than val in keys() [ENH] in all_objects changing filter_tags by str and "list of str" to mean {val: True} rather than val in keys() May 14, 2024
@fkiraly fkiraly mentioned this issue May 25, 2024
Closed
fkiraly added a commit that referenced this issue May 25, 2024
@fkiraly
[ENH] all_objects retrieval filtered by regex applied to tag values…
18dff9c 
…, deprecation of "has tag" condition in favour of "tag is True" (#329)

This PR closes #326.

It implements:

* functionality for `all_objects` to retrieve classes by specifying
regex for tag values
* deprecation warnings for `str` and `list of str` condition meaning to
change from "has tag" to "tag is True" meaning, from 0.9.0 release
onwards
* the deprecation message includes instructions on how to retain current
behaviour - this uses the regex specification

This PR also improves formatting of the docstring of `all_objects`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement Adding new functionality implementing framework Implementing core skbase framework
Projects
None yet
Milestone
No milestone
1 participant
@fkiraly