Updated Readme #1685
Updated Readme #1685
Conversation
There was a problem hiding this comment.
Thanks @davemfish, this is a vast improvement! Just had a few small suggestions.
| In practice, it can be convenient to use an "editable install" instead to avoid needing | ||
| to re-install after making changes to source code:: | ||
|
|
||
| $ pip install -e . |
There was a problem hiding this comment.
Yes! Took me weeks to figure this out 🤣
There was a problem hiding this comment.
It took me years!
|
|
||
| $ make install | ||
|
|
There was a problem hiding this comment.
I would add a note here about the need to uninstall before reinstalling. Something along these lines:
To update your local installation after making changes to source code::
$ pip uninstall natcap-invest
$ make install
This'll get you unstuck if you (intentionally or unintentionally) use make install, and it underscores why an editable install (described next) is so compelling.
There was a problem hiding this comment.
I almost added this. For a long time my routine was pip uninstall -y && make install but then I questioned whether the uninstall was actually necessary. But I think you're right, sometimes it is and it never hurts, so this should be described here.
README.rst
Outdated
|
|
||
| $ pip install -e . | ||
|
|
||
| But note that any changes to non-Python files will still require compilation. |
There was a problem hiding this comment.
I think it would be worth explaining what this means in practice, i.e., if I've changed non-Python files, do I need to uninstall and reinstall natcap.invest?
README.rst
Outdated
|
|
||
| $ pip install -e . | ||
|
|
||
| But note that any changes to non-Python files will still require compilation. |
There was a problem hiding this comment.
I would also add a parenthetical: non-Python files (excluding the workbench), just to avoid confusion.
There was a problem hiding this comment.
I'm adding a note to explain that the workbench is not part of the natcap.invest python library.
README.rst
Outdated
| @@ -145,6 +153,13 @@ To build the user's guide:: | |||
| This will build HTML and PDF documentation, writing them to ``dist/userguide`` | |||
| and ``dist/InVEST_*_Documentation.pdf``, respectively. | |||
|
|
|||
| The User's Guide is maintained in a separate git reporsitory. InVEST will build | |||
There was a problem hiding this comment.
It is admittedly fun to say reporsitory, but we should probably fix the typo anyway. 😃
Thanks for the suggestions, @emilyanndavis. I pushed some changes. |
README.rst
Outdated
| manage your own virtual environment. ``requirements.txt``, | ||
| ``requirements-dev.txt`` and ``requirements-docs.txt`` list the python | ||
| dependencies needed. | ||
| This makefile target is included for convenience. It uses ``conda`` and installs packages from ``conda-forge``. | ||
|
|
||
| Using a different environment name | ||
| """""""""""""""""""""""""""""""""" |
There was a problem hiding this comment.
Would it be more correct to use folder or directory instead of name as the environment is referenced by its path rather than name? I.e., conda activate ./env rather than conda activate env. Maybe this is a me problem but the only way I could get a named environment was to specify ENV=path/to/miniconda/base /myEnv
There was a problem hiding this comment.
Yes, I think you're right. Since the env target of the Makefile calls conda like this:
$(CONDA) create -p $(ENV)
It's using the -p flag, so ENV will always be interpreted as a path to a directory.
Maybe this is a me problem but the only way I could get a named environment was to specify ENV=path/to/miniconda/base /myEnv
That makes sense; not a you problem. I guess make env was designed with the opinion that prefixed environments are a good idea. But creating our own environments in whatever way we prefer as individuals is also a good idea!
Added notes on Python environment and
natcap.investinstallation.Added notes on how we link to external repos, sample-data, test-data, and User's Guide.
Some other small sections were removed because they were out of date.
Fixes #670
It looks like all the tests are failing for an unrelated reason (see #1686 ) so I'm not going to address that in this PR.
Checklist