Hide nbdev import - Githubissues

albertvillanova commented 4 years ago

Hide the imports of nbdev.

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

Check out this pull request on

Review Jupyter notebook visual diffs & provide feedback on notebooks.

Powered by ReviewNB

jph00 commented 4 years ago

Thanks that's a good idea. Please remove the changes not related to hiding nbdev from this PR. We only want one issue handled per PR.

albertvillanova commented 4 years ago

There were cells where the import of nbdev was not alone. In those cases, I just moved the other imports to the precedent cell. These are the only additional changes.

jph00 commented 4 years ago

Have a look at the diff - there's quite a few changes to what's being exported.

albertvillanova commented 4 years ago

I understand and I am going to fix them. Nevertheless, there is one case where I think the new export is necessary:

Your previously mentioned second case corresponds to the notebook nbs/00_torch_core.ipynb: https://github.com/fastai/fastai2/pull/530/files#diff-b1e719d4fd7782e7ac23d27b467c08be
- The cell containing the nbdev import, also contained from PIL import Image
- I have moved this import to the precedent cell
- Note that the precedent cell is exported to the library file torch_core.py
- Also note that Image is required in the library file torch_core.py, in the definition of the function to_image(x): https://github.com/fastai/fastai2/blob/2a4de50325c94b09561303cf8092bc636ee13c99/fastai2/torch_core.py#L654
- Currently this generates an Unresolved reference error
- My PR fixes this

I am removing all changes other than hiding nbdev from this PR and I will eventually create another PR to fix the Unresolved reference errors.

jph00 commented 4 years ago

Many thanks!

fastai / fastai2

Hide nbdev import #530