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

FIX EmptyRequest.get defaults to Bunch of METHODS #28371

Merged
merged 13 commits into from
Feb 13, 2024
Merged

FIX EmptyRequest.get defaults to Bunch of METHODS #28371

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
5e94bd1
fix(routing): `EmptyRequest.get` defaults to `Bunch` of `METHODS`
eddiebergman Feb 6, 2024
e56b4d8
test: Preventitive regression with `_MultimetricScorer` and
eddiebergman Feb 6, 2024
6da316e
test: Move test to `test_score_objects.py`
eddiebergman Feb 6, 2024
2aa6c16
doc: Make test name and description clearer about what's tested
eddiebergman Feb 6, 2024
940e53d
fix: Reset changes to `test_metadata_routing`
eddiebergman Feb 7, 2024
8ed5630
test: Parametrize test
eddiebergman Feb 9, 2024
4bcaab6
test: Add test for `get()` behaviour on returned value of
eddiebergman Feb 9, 2024
6610d7b
style: Fix linting with ruff
eddiebergman Feb 9, 2024
d32ca3a
test: Fixup
eddiebergman Feb 9, 2024
d500ae5
fix(routing): Use sentinel for `EmptyRequest.get`
eddiebergman Feb 12, 2024
974c559
fix: Use the sentinel value... whoops
eddiebergman Feb 12, 2024
adf4eff
fix(metadata): EmptyRequest ignores default in `get`
eddiebergman Feb 12, 2024
2ca2aa7
test: Parametrized test
eddiebergman Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion sklearn/utils/_metadata_requests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1530,7 +1530,10 @@ def process_routing(_obj, _method, /, **kwargs):
# an empty dict on routed_params.ANYTHING.ANY_METHOD.
class EmptyRequest:
def get(self, name, default=None):
return default if default else {}
if not default:
return Bunch(**{method: dict() for method in METHODS})

return default
Copy link
Member

Choose a reason for hiding this comment

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

codecov is not happy here. I need to figure out when is it the case that default=None

Copy link
Member

Choose a reason for hiding this comment

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

@adrinjalali I assume that we should be able to cover this one because it would be equivalent to call e.g.

routed_params = _process_routing(self, "score", **kwargs)
routed_params.get("score", default="default")

I don't where is the best place to test this. This looks like a metadata routing test to me.


def __getitem__(self, name):
return Bunch(**{method: dict() for method in METHODS})
Expand Down