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

Modify init to pull the collect_env libraries #1994

Merged
merged 5 commits into from
Apr 23, 2019
Merged

Modify init to pull the collect_env libraries #1994

merged 5 commits into from
Apr 23, 2019

Conversation

kevinbird15
Copy link
Contributor

This will allow somebody to the show_install function with from fastai.utils import * instead of needing to remember that it is in collect_env

Here is the forum thread discussing this change: https://forums.fast.ai/t/show-install-modification-suggestion/44214

This will allow somebody to the show_install function with `from fastai.utils import *` instead of needing to remember that it is in collect_env
@sgugger
Copy link
Contributor

sgugger commented Apr 21, 2019

Seems consistent with the rest of what we have. @stas00 any reason not to do this?

@stas00
Copy link

stas00 commented Apr 21, 2019

I have no objection, but if you do that, please adjust the docs, otherwise only you will know that there is a different way.

@sgugger
Copy link
Contributor

sgugger commented Apr 22, 2019

Would you mind adding the proper documentation in this PR @kevinbird15 ?

@kevinbird15
Copy link
Contributor Author

Sure, I will get that added today sometime.

@stas00
Copy link

stas00 commented Apr 22, 2019

Just to clarify, there is only a need to edit the existing docs:

docs/performance.md:python -c "import fastai.utils.collect_env; fastai.utils.collect_env.check_perf()"
docs/support.md:   python -c 'import fastai.utils.collect_env; fastai.utils.collect_env.show_install(1)'
docs/troubleshoot.md:python -c 'import fastai.utils.collect_env; fastai.utils.collect_env.show_install(1)'
docs/troubleshoot.md:python -c 'import fastai.utils.collect_env; fastai.utils.collect_env.show_install(1)'
docs_src/utils.collect_env.ipynb:    "from fastai.utils.collect_env import *"

Thank you.

@kevinbird15
Copy link
Contributor Author

Ok, so would you change it to just be fastai.utils and remove the collect_env part then or just note that there is another way to do it? So for each of these:

docs/performance.md:python -c "import fastai.utils; fastai.utils.check_perf()"
docs/support.md:   python -c 'import fastai.utils; fastai.utils.show_install(1)'
docs/troubleshoot.md:python -c 'import fastai.utils; fastai.utils.show_install(1)'
docs/troubleshoot.md:python -c 'import fastai.utils; fastai.utils.show_install(1)'
docs_src/utils.collect_env.ipynb:    "from fastai.utils import *"

Remove the need for .collect_env from docs as this actually works with just fastai.utils at this point.
Remove the need for .collect_env from docs as this actually works with just fastai.utils at this point.
Remove the need for .collect_env from docs as this actually works with just fastai.utils at this point.
Remove the need for .collect_env from docs as this actually works with just fastai.utils at this point.
@kevinbird15
Copy link
Contributor Author

Ok, probably terrible practice, but I added commits to change these docs. I would have rather put all of the commits into one file, but I did it with the online edit file functionality. It seems like it worked ok, but I would figure out how to do it on my local version next time and push the changes up instead. Quick and dirty won me over this time though

@sgugger
Copy link
Contributor

sgugger commented Apr 23, 2019

We don't mind quick and dirty, I know how painful it can be to checkout your PR branch and add commits when not used to git :)
Thanks!

@sgugger sgugger merged commit f4f4255 into fastai:master Apr 23, 2019
@kevinbird15 kevinbird15 deleted the patch-3 branch April 23, 2019 13:25
@stas00
Copy link

stas00 commented Apr 23, 2019

I know how painful it can be to checkout your PR branch and add commits when not used to git :)

FYI, it's trivial to do if you use fastai-make-pr-branch. It does all the work for you.

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.

3 participants