[MRG + 1] Fix for image.extract_patches_2d when # of patches requested > all possible patches #10101
Conversation
varunagrawal
changed the title
image.extract_patches_2d when # of patches requested > all possible patches 
|
please add a test |
|
Added 2 tests |
| @@ -162,6 +162,7 @@ def test_extract_patches_max_patches(): | |||
|
|
|||
| expected_n_patches = int(0.5 * (i_h - p_h + 1) * (i_w - p_w + 1)) | |||
| patches = extract_patches_2d(face, (p_h, p_w), max_patches=0.5) | |||
|
|
|||
| i_h, i_w = face.shape | ||
|
|
||
| # Request patches of the same size as image | ||
| # Should return just the single patch a.k.a. the image |
There was a problem hiding this comment.
I would like probably to assert the number of returned patch
| def test_extract_patches_less_than_max_patches(): | ||
| face = downsampled_face | ||
| i_h, i_w = face.shape | ||
| p_h, p_w = int(3*i_h/4), int(3*i_w/4) # get patches of 3/4th the image size |
There was a problem hiding this comment.
Can you put some space between the mathematical operator and you don't need for any comment. This is straightforward
There was a problem hiding this comment.
You should also used python 3 standard without casting
p_h, p_w = 3 * i_h // 4, 3 * i_w // 4There was a problem hiding this comment.
Just add at the beginning of the file
from __future__ import divisionThere was a problem hiding this comment.
Awesome! I was trying to be backwards compatible. Thank you!
varunagrawal
force-pushed
the
sample-patches-fix
branch
3 times, most recently
from
4779bc9 to
19ced42
Compare
varunagrawal
changed the title
|
just don't forget to update what's new |
|
@agramfort can you please elaborate some more on your last comment? I seem to have lost the context in which it was made. |
|
You need to document the change in the whats_new.rst page |
Codecov Report
@@ Coverage Diff @@
## master #10101 +/- ##
==========================================
+ Coverage 96.1% 96.1% +<.01%
==========================================
Files 337 337
Lines 63295 63309 +14
==========================================
+ Hits 60828 60842 +14
Misses 2467 2467
Continue to review full report at Codecov.
|
varunagrawal
force-pushed
the
sample-patches-fix
branch
from
cd24992 to
b98b42f
Compare
|
@agramfort thank you for the info. I have updated the document. |
doc/whats_new/v0.20.rst
Outdated
| @@ -175,6 +175,14 @@ Metrics | |||
| :issue:`10093` by :user:`alexryndin <alexryndin>` | |||
| and :user:`Hanmin Qin <qinhanmin2014>`. | |||
|
|
|||
| Feature Extraction | |||
|
|
|||
| - Fixed a bug in :func:`feature_extraction.image._compute_n_patches` where an exception | |||
There was a problem hiding this comment.
you cannot point to a private function feature_extraction.image._compute_n_patches
make sure doc renders fine
|
@varunagrawal could you solve the conflict and address @agramfort comments |
varunagrawal
force-pushed
the
sample-patches-fix
branch
from
b98b42f to
424c447
Compare
|
@glemaitre I have made the requested changes. Apologies for the delay. |
varunagrawal
force-pushed
the
sample-patches-fix
branch
from
424c447 to
e53ae2b
Compare
|
don't document changes in private function. Say what public function is impacted. btw see how doc renders: https://15080-843222-gh.circle-artifacts.com/0/home/ubuntu/scikit-learn/doc/_build/html/stable/whats_new.html |
varunagrawal
force-pushed
the
sample-patches-fix
branch
from
e53ae2b to
f19b1e1
Compare
|
@agramfort sorry about that. Still getting used to the way things are done here. |
glemaitre
changed the title
|
Merging since only what's new was pending. Thanks for your contribution. |
…d > all possible patches (scikit-learn#10101)
Reference Issues/PRs
Fixes #10100
What does this implement/fix? Explain your changes.
This updates the
_compute_n_patchesfunction to check if the max_patches value >= all_patches and return the appropriate value for number of patches.Any other comments?