8000 ENH Include entire range in check_scalar error message by MaxwellLZH · Pull Request #22721 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

ENH Include entire range in check_scalar error message #22721

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

Closed
wants to merge 2 commits into from

Conversation

MaxwellLZH
Copy link
Contributor

Reference Issues/PRs

This is a PR to address issue #22691 , where the entire boundary will be shown in the error message within check_scalar.

Copy link
Member
@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!

Note that many unit tests will need to change since they check against older string.

f"{name} == {x}, must be"
f" {'<=' if include_boundaries in ('right', 'both') else '<'} {max_val}."
)
raise ValueError(f"{name} == {x}, must be in the range {boundary}.")
Copy link
Member
@thomasjpfan thomasjpfan Mar 7, 2022

Choose a reason for hiding this comment

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

Nit: I prefer to avoid creating the string if there is no error:

    include_left = include_boundaries in ("left", "both")
    include_right = include_boundaries in ("right", "both")
    left_comparison_operator = operator.lt if include_left else operator.le
    right_comparison_operator = operator.gt if include_right else operator.ge

    if (min_val is not None and left_comparison_operator(x, min_val)) or (
        max_val is not None and right_comparison_operator(x, max_val)
    ):
        # Assume that inf boundaries are always open
        left_bound = "[" if include_left and min_val is not None else "("
        right_bound = "]" if include_right and max_val is not None else ")"
        min_val_str = "-inf" if min_val is None else min_val
        max_val_str = "inf" if max_val is None else max_val

        range_str = f"{left_bound}{min_val_str}, {max_val_str}{right_bound}"
        raise ValueError(f"{name} == {x}, must be in range {range_str}.")

(Also to avoid the nested if statement in the current implementation)

@MaxwellLZH MaxwellLZH closed this Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0