fastaidocsprint / fastai

Documentation Sprint for the fastai deep learning library
http://fastaidocsprint.github.io/fastai
Apache License 2.0
15 stars 17 forks source link

docment resize crop pad #81

Closed dienhoa closed 2 years ago

dienhoa commented 2 years ago

Docment

in vision.augment, issue: https://github.com/muellerzr/fastai-docment-sprint/issues/29

review-notebook-app[bot] commented 2 years ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

dienhoa commented 2 years ago

I commit a new version of this PR with your suggestions @muellerzr. Thanks.

marii-moe commented 2 years ago

@dienhoa I noticed you added split_idx=1 to this code cropped = crop(img, split_idx=1)

This is stopping the example images from being augmented.

dienhoa commented 2 years ago

@dienhoa I noticed you added split_idx=1 to this code cropped = crop(img, split_idx=1)

This is stopping the example images from being augmented.

Oh sorry, I did some experiments and forget to clean the notebook. Will change it back to the original

dienhoa commented 2 years ago

Thanks for your detailed explanation @marii-moe. I commit a new version with suggestions

dienhoa commented 2 years ago

github question. If now I rebase my branch with @muellerzr master to resolve conflict, does it pollute everything we've done here?

muellerzr commented 2 years ago

If you rebase it should make you address those conflicts I think

dienhoa commented 2 years ago

I solved the conflicts and have some notes:

marii-moe commented 2 years ago

@dienhoa There is this to try: https://nbdev.fast.ai/merge.html#nbdev_fix_merge

Otherwise I can take a look at it tomorrow.

dienhoa commented 2 years ago

@dienhoa There is this to try: https://nbdev.fast.ai/merge.html#nbdev_fix_merge

Otherwise I can take a look at it tomorrow.

I did run nbdev_fix_merge already. The conflict is solved in my last commit (you can see that all CI jobs passed). Just my thinking that we will always have conflicts because the output image always change

marii-moe commented 2 years ago

@dienhoa sorry I misunderstood. I will go ahead and merge then.