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

[DOCS] Change some tutorial text #6514

Merged
merged 4 commits into from Sep 22, 2020
Merged

Conversation

rkimball
Copy link
Contributor

I had a few issues following the how to install from source on linux so I addressed them here.

@rkimball
Copy link
Contributor Author

@tqchen

docs/install/from_source.rst Outdated Show resolved Hide resolved
@@ -205,7 +205,7 @@ like ``virtualenv``.

.. code:: bash

pip3 install --user numpy decorator attrs
pip3 install --user numpy decorator attrs typed_ast pytest
Copy link
Contributor

Choose a reason for hiding this comment

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

According to python/setup.py, the complete list should also include scipy and psutil, but not pytest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was following the instructions literally on a new ubuntu 20.04 install and these are the changes I needed to make to get the example to work.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Which example did you try? In general if users only want to use instead of developing TVM, we should not ask them to install unnecessary packages such as pytest and mypy.

IMHO, we could indicate an example at the end of this doc to make it clear. It might be better to also add descriptions about installing extra requirements for developments (e.g., testing).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps we can add a new section

  • If you want to run unit tests
    pip3 install --user ...

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a good idea to me 👍

Copy link
Contributor Author

@rkimball rkimball Sep 18, 2020

Choose a reason for hiding this comment

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

I reverted this change. To do this right I would need to start with a new clean linux install and go through the process to figure out what is needed for running unit tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, we should eventually fix this properly with an official requirements.txt or setup.py set of dependencies, so people don't have to roll their own lists.

Copy link
Contributor

Choose a reason for hiding this comment

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

@rkimball would you prefer to add the unit test section in this PR or file a follow-up PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think follow up PR to do what @binarybana suggests

@tqchen tqchen changed the title Change some tutorial text [DOCS] Change some tutorial text Sep 18, 2020
@tqchen tqchen merged commit 1253b52 into apache:master Sep 22, 2020
@tqchen
Copy link
Member

tqchen commented Sep 22, 2020

thanks @rkimball @comaniac @binarybana

TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 13, 2020
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 14, 2020
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 15, 2020
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 16, 2020
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants