Skip to content
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

added docments for 09c_vision.widgets.ipynb #82

Merged
merged 15 commits into from
Mar 11, 2022

Conversation

omer-dor
Copy link

fixes issue #31

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@tyoc213 tyoc213 self-requested a review February 24, 2022 02:32
fastai/vision/widgets.py Outdated Show resolved Hide resolved
fastai/vision/widgets.py Outdated Show resolved Hide resolved
fastai/vision/widgets.py Outdated Show resolved Hide resolved
fastai/vision/widgets.py Outdated Show resolved Hide resolved
fastai/vision/widgets.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@muellerzr muellerzr left a comment

Choose a reason for hiding this comment

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

Added a few nits as a first review

omer-dor and others added 8 commits February 25, 2022 23:33
Updated carusel function docment

Co-authored-by: Zachary Mueller <[email protected]>
Updated _get_iw_info docments

Co-authored-by: Zachary Mueller <[email protected]>
Update ds_idx in docment in  _get_iw_info function

Co-authored-by: Zachary Mueller <[email protected]>
Update on_change_ds docment

Co-authored-by: Zachary Mueller <[email protected]>
@omer-dor
Copy link
Author

Hi! I hope I fixed the issues mentioned. Waiting for another feedback :)

@omer-dor omer-dor requested a review from muellerzr February 26, 2022 17:05
@muellerzr
Copy link
Collaborator

LG2M! Will let @tyoc213 review as well :)

@tyoc213
Copy link
Collaborator

tyoc213 commented Mar 4, 2022

@omer-dor the only change I see is that README should not be modified, the other 3 updated files lg2m

README.md Outdated Show resolved Hide resolved
@omer-dor omer-dor requested a review from tyoc213 March 4, 2022 06:58
for o in change['owner'].children:
if not o.layout.flex: o.layout.flex = '0 0 auto'

# Cell
def carousel(children=(), **layout):
def carousel(
children:tuple=(), # `Box` objects to display in a carousel
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think children is limited to tuple but can be a list too.

@@ -38,32 +44,55 @@ def carousel(children=(), **layout):
return res

# Cell
def _open_thumb(fn, h, w): return Image.open(fn).to_thumb(h, w).convert('RGBA')
def _open_thumb(
fn, # A path of an image
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
fn, # A path of an image
fn:(Path, str), # A path of an image


# Cell
class ImagesCleaner:
"A widget that displays all images in `fns` along with a `Dropdown`"
def __init__(self, opts=(), height=128, width=256, max_n=30):
def __init__(self,
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing type hints

Copy link
Author

Choose a reason for hiding this comment

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

Thanks! I added them

fn, # A path of an image
h:int, # Thumbnail Height
w:int # Thumbnail Width
)-> Image: # `PIL` image to display
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
)-> Image: # `PIL` image to display
) -> Image: # `PIL` image to display

"A widget that provides an `ImagesCleaner` with a CNN `Learner`"
def __init__(self, learn, **kwargs):
"A widget that provides an `ImagesCleaner` for a CNN `Learner`"
def __init__(self,learn,**kwargs):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def __init__(self,learn,**kwargs):
def __init__(self, learn, **kwargs):

@@ -17,19 +17,25 @@
def __getitem__(self:Box, i): return self.children[i]

# Cell
def widget(im, *args, **layout):
"Convert anything that can be `display`ed by IPython into a widget"
def widget(im,*args,**layout)-> Output:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def widget(im,*args,**layout)-> Output:
def widget(im, *args, **layout) -> Output:

@omer-dor omer-dor requested a review from tmabraham March 7, 2022 06:42
Copy link
Collaborator

@tmabraham tmabraham left a comment

Choose a reason for hiding this comment

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

LGTM

@tmabraham tmabraham merged commit cc9034d into fastaidocsprint:master Mar 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants