Skip to content

FIX avoid scalar/array boolean operation in _IffHasAttrDescriptor #21145

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
Merged
merged 5 commits into from
Oct 20, 2021
Merged

FIX avoid scalar/array boolean operation in _IffHasAttrDescriptor #21145

merged 5 commits into from
Oct 20, 2021

Conversation

Zahlii
Copy link
Contributor

@Zahlii Zahlii commented Sep 24, 2021

Reference Issues/PR

Fixes #21144

What does this implement/fix? Explain your changes.

Any other comments?

@ogrisel ogrisel changed the title Fix #21144 Fix _IffHasAttrDescriptor when attribute exists Sep 24, 2021
@ogrisel
Copy link
Member

ogrisel commented Sep 24, 2021

Thanks for the PR. Could you please try to contribute an new non-regression test for this?

@Zahlii
Copy link
Contributor Author

Zahlii commented Sep 24, 2021

@ogrisel If you can point me to where this code could be placed, sure. I assume I could add a completely new test case to tests/test_metaestimators.py that essentially mimics the code from #21144 ?

@ogrisel
Copy link
Member

ogrisel commented Sep 27, 2021

@ogrisel If you can point me to where this code could be placed, sure. I assume I could add a completely new test case to tests/test_metaestimators.py that essentially mimics the code from #21144 ?

Sounds good to me.

@adrinjalali
Copy link
Member

@Zahlii would you be able to continue this PR? We'll need it for the next minor release.

@adrinjalali adrinjalali added this to the 1.0.1 milestone Oct 12, 2021
@glemaitre
Copy link
Member

@Zahlii The code that you pointed out is a good regression case. We will also have an entry for v 1.0.1

@glemaitre glemaitre self-assigned this Oct 20, 2021
@glemaitre glemaitre changed the title Fix _IffHasAttrDescriptor when attribute exists FIX avoid scalar/array boolean operation in _IffHasAttrDescriptor Oct 20, 2021
@glemaitre glemaitre removed their assignment Oct 20, 2021
glemaitre
glemaitre approved these changes Oct 20, 2021
View reviewed changes
Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

I added a non-regression test and an entry in what's new.

adrinjalali
adrinjalali approved these changes Oct 20, 2021
View reviewed changes
Copy link
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

LGTM!

@adrinjalali
Copy link
Member

Feel free to merge once CI passes.

@jeremiedbb
Copy link
Member

Thanks @Zahlii !

@jeremiedbb jeremiedbb merged commit 9cf2465 into scikit-learn:main Oct 20, 2021
@glemaitre glemaitre mentioned this pull request Oct 23, 2021
Merged
10 tasks
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Oct 23, 2021
@Zahlii @glemaitre
FIX avoid scalar/array boolean operation in _IffHasAttrDescriptor (sc…
1d17f94 
…ikit-learn#21145)
glemaitre pushed a commit that referenced this pull request Oct 25, 2021
@Zahlii @glemaitre
FIX avoid scalar/array boolean operation in _IffHasAttrDescriptor (#2…
896ad12 
…1145)
samronsin pushed a commit to samronsin/scikit-learn that referenced this pull request Nov 30, 2021
@Zahlii @samronsin
FIX avoid scalar/array boolean operation in _IffHasAttrDescriptor (sc…
169a8c5 
…ikit-learn#21145)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrinjalali adrinjalali adrinjalali approved these changes

@glemaitre glemaitre glemaitre approved these changes

Assignees
No one assigned
Labels
module:utils
Projects
None yet
Milestone
1.0.1
Development

Successfully merging this pull request may close these issues.

_IffHasAttrDescriptor fails if attribute exists, but cannot be coerced to a truthy value
5 participants
@Zahlii @ogrisel @adrinjalali @glemaitre @jeremiedbb