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

doc for 43_tabular.learner #61

Closed moarshy closed 2 years ago

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'm not a mentor but I think there are some points that might need to improve in this PR.

I see there are 9 files changed. Normally if you want to docment for 43_tabular.learner, I think there should have only one .ipynb file, one .py file and one .HTML file.

I think we should only commit these 3 files for the PR.

You can check out my note about useful commands that I use during this document sprint. https://gist.github.com/dienhoa/547664c51940906556a15a08cef70e54

Thanks for participating!

moarshy commented 2 years ago

I'm not a mentor but I think there are some points that might need to improve in this PR.

I see there are 9 files changed. Normally if you want to docment for 43_tabular.learner, I think there should have only one .ipynb file, one .py file and one .HTML file.

I think we should only commit these 3 files for the PR.

You can check out my note about useful commands that I use during this document sprint. https://gist.github.com/dienhoa/547664c51940906556a15a08cef70e54

Thanks for participating!

Thank you for the feedback. Yes, I see other PRs only have 3 files changed. Not sure what Im doing wrong. Will take a look. Btw, Im using colab, not sure that's creating the difference.

dienhoa commented 2 years ago

Try to run nbdev_build_doc with the only corresponding file: nbdev_build_docs —fname {path the to file}. then running nbdev_clean_nbs.

For the README, I also have modification but I don't add it to the commit

moarshy commented 2 years ago

Thank you. I am doing nbdev_build_docs —fname {path the to file} already . I might have caught my mistake I ran nbdev_clean_nbs first followed by the other two. Will see what the mentors advise.

moarshy commented 2 years ago

A few nits, please bring back the README file. It also seems like extraneous CSS's and whatnot were brought in and/or deleted. Please try and fix this

Understand there should be only three files committed (.ipynb, .py, .html of the nbs I edited). In my case, I was wondering how can I rectify this? Thanks a lot

muellerzr commented 2 years ago

https://stackoverflow.com/questions/39459467/remove-a-modified-file-from-pull-request

muellerzr commented 2 years ago

@moarshy one small nit and then we can merge, we have 2x the number of TabularModel and tabular_learner's. Please pick one to keep 😉

moarshy commented 2 years ago

@muellerzr r u ok with this? :)

muellerzr commented 2 years ago

Thanks! Great work!