-
- Notifications
You must be signed in to change notification settings - Fork 26.3k
FIX SLEP6 no errors raised when no metadata is passed #28256
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
Conversation
I am not sure that we want to fix it. Until we are experimental, I think that we don't want people to be able to have a mixture of supported and unsupported estimator. |
But if they don't pass any metadata, it doesn't really matter. |
That's true that we are not allowing for experimenting while you don't use the feature. So this is probably best to be lenient. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I'm ok going forward.
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…8256) Co-authored-by: Loïc Estève <loic.esteve@ymail.com> Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
…8256) Co-authored-by: Loïc Estève <loic.esteve@ymail.com> Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Loïc Estève <loic.esteve@ymail.com> Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Fixes #28246
I'm not sure if this should be fixed though. But the fix is pretty trivial.
WDYT @OmarManzoor @glemaitre @thomasjpfan