Skip to content

DOC use integer to encode the target #25130

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 1 commit into from

Conversation

glemaitre
Copy link
Member

Fix the documentation CIs.

However, we introduced a regression: a target array could be of any type and pos_label should reflect that, I assume.

@glemaitre
Copy link
Member Author

So in mind, I think that we should support as well:

  • bool
  • floating

We can fix those in a subsequent PR just to have the documentation CI working for the moment.

ping @jeremiedbb @thomasjpfan

@thomasjpfan
Copy link
Member

For reference, here is the failure. I think the example failure is by design and should not change because we introduced a bug. I prefer fixing the issue by changing the constraints for pos_label.

If it's urgent to get the CI passing, then I would revert #25108 and follow up with a PR to fix it.

@jeremiedbb
Copy link
Member

I'm going to open a PR to fix it. I wonder if we should put "no_validation or [Real, str, "boolean"] is enough ?

@glemaitre
Copy link
Member Author

I think [Real, str, "boolean"] should be enough. I don't see an additional valid type. In this manner, we should also update the docstring.

@glemaitre
Copy link
Member Author

Looking at the problem again, I would have expected the validation to magically work because isinstance(True, numbers.Integral) is True. I probably missed something in the validation function.

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.

3 participants