Skip to content

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