Skip to content

Point to existing install instructions #511

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

Merged
merged 3 commits into from
Dec 26, 2018
Merged

Conversation

mkcor
Copy link
Contributor

@mkcor mkcor commented Dec 22, 2018

Hello @plotly/dash @rmarren1 and @T4rk1n!

I'm trying to document the installation of dash-bio, so I came here for inspiration! In the current version of the README, there are no installation (let alone development) instructions. That's why I'm suggesting pointing to the user guide.

@T4rk1n T4rk1n mentioned this pull request Dec 24, 2018
CONTRIBUTING.md Outdated
@@ -2,7 +2,7 @@

## Getting Started

Refer to the [readme](README.md) for installation and development instructions.
Refer to the Dash User Guide for [installation](https://dash.plot.ly/installation) instructions.
Copy link
Contributor

Choose a reason for hiding this comment

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

The installation instructions on the dash docs is for developing dash apps.
Installation for development on the dash repo should be:

  • Create a virtualenv:
    python -m venv venv or virtualenv venv for python 2.
  • Activate the virtualenv:
    . venv/bin/activate or venv\scripts\activate for windows.
  • Install the dev dependencies:
    pip install -r .circleci/requirements/dev-requirements.txt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reviewing, @T4rk1n! In light of your comment, I have submitted another change, which is more to the point. I don't think it is healthy to promote the use of Python 2 any more (I'll share references if you ask for them), so I have only included the Python 3 way.

CONTRIBUTING.md Outdated
# Create a virtualenv (name it, say, dash-dev)
python3 -m venv dash-dev
# Activate this virtualenv
. dash-dev/bin/activate
Copy link
Contributor

Choose a reason for hiding this comment

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

From these instructions it is not clear where the virtual environment will reside. Complete setup instruction should include:
$ git clone https://github.com/plotly/dash.git
$ cd dash
$ python -m venv venv
...

venv name is normally used in the project directory, it is ignored in .gitignore/.npmignore of most projects. I only name my environments when I work multi-repo and I put it in another folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I can see venv in the .gitignore file; I wasn't aware of this convention; I used to keep all my virtualenvs under ~/.virtualenv/ (calling for prior command mkdir -p ~/.virtualenv/ as in http://nipy.org/nipy/devel/tools/virtualenv-tutor.html).

Lately, I have used almost only conda environments, following the wave in the SciPy/PyData community, so thanks for the refresher! I'll make and push the change right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@T4rk1n hmm, in the general case, newcomers willing to contribute would have to fork the repo first... I'll replace the first command with a generic comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not to mention that some people (including me) would clone over SSH ;)

@mkcor
Copy link
Contributor Author

mkcor commented Dec 26, 2018

Voilà, @T4rk1n! I have used your convention of prefixing commands with $ :)

@T4rk1n
Copy link
Contributor

T4rk1n commented Dec 26, 2018

Looking good, thanks 💃

@T4rk1n T4rk1n merged commit f9820fd into master Dec 26, 2018
@mkcor mkcor deleted the mkcor-install-instructions branch December 26, 2018 22:11
AnnMarieW pushed a commit to AnnMarieW/dash that referenced this pull request Jan 6, 2022
…ate-callback

Issue 510 fix clearing date callback
AnnMarieW pushed a commit to AnnMarieW/dash that referenced this pull request Jan 6, 2022
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.

2 participants