Documentation geoips installation #651

biosafetylvl5 · 2024-06-26T20:42:35Z

Related Issues

fixes #609
related to #611 and #531

Testing Instructions

Build the docs 🙂

Summary

I updated the install docs. They were tested line-by-line on a mac and a debian system for conda, expert and docker installs.

Output

All done:

Return values:
  Python install: 
  sphinx html:    0
  sphinx latex:   None
  latex pdf make: None
  warnings html:  
  warnings latex: 
  warnings make:  

Final retval: 0

Relevant XKCD: (Universal Install Script)

…ges are also needed

…installation

mindyls · 2024-07-22T16:13:09Z

docs/source/new-docs/getting-started/installing/index.rst

+
+.. code:: bash
+
+    mamba activate geoips


I think conda activate geoips all the time - mamba activate doesn't seem to actually work.

Good point - changed

mindyls · 2024-07-22T16:14:39Z

docs/source/new-docs/getting-started/installing/index.rst

+Install system dependencies directly
+------------------------------------
+
+If you are an expert user, you may want to install the dependencies manually. Do not do this if you already installed
+dependencies with Conda.
+
+Required
+^^^^^^^^
+
+* ``git``
+* ``openblas`` (required for scipy pip install)
+* ``make`` (required for pypublicdecompwt)
+* ``python`` >= 3.9
+
+Optional
+^^^^^^^^
+
+* ``gfortran`` (required for plugins including fortran builds)
+* ``gcc`` and ``g++`` (required for plugins including fortran or C builds, and ARM machines)
+* ``pdflatex`` (optional, for building pdf documentation)
+* ``wget`` (required for downloading test data)
+* ``make`` (optional, for building packages on ARM machines)
+* Test data repos can be installed in `$GEOIPS_TESTDATA_DIR`


I still think this should not go in the standard installation. We should keep the standard installation very clean and vanilla, to avoid unnecessary confusion. Our standard supported installation is conda - and this is not necessary if you are installing with conda.

I would just remove this entire section

To echo Mindy, I also agree that the standard installation instructions (conda install on a Linux machine) should be clean and as minimal as possible. I think it not only makes it easier for the end-user, but also makes it easier for us to help troubleshoot a standard installation if needed.

Great! I changed this document to be conda only 🙂

mindyls · 2024-07-22T16:15:25Z

docs/source/new-docs/getting-started/installing/index.rst

+    # Ensure geoips python environment enabled before installing geoips
+    pip install "$GEOIPS_PACKAGES_DIR/geoips[doc,lint,test,debug]"
+
+The optional dependencies are:


"All of the optional dependencies are required in order for tests to pass"

Is that true for debug, lint and doc?

My install is passing base_test with only test

And I'm not passing the pytest with or without the lint/doc/debug

build_docs.sh is part of the "test_all.sh" scripts in geoips_clavrx and data_fusion, and full_test.sh to pass. If we want people to use the test_all.sh and/or full_test.sh to confirm full functionality, then it does include doc and lint. Probably not debug

mindyls · 2024-07-22T16:16:07Z

docs/source/new-docs/getting-started/installing/index.rst

+Development
+-----------
+
+The installation steps for developers are the same as for normal and/or expert users, except for one step.
+
+Most developers use Conda installations, but any dependency management solution is fine.
+
+When installing geoips, please install all the extras and install in **editable** mode so that changes to the code are
+immediately reflected in the installed package. Eg.
+
+.. code:: bash
+
+    # Ensure geoips python environment enabled before installing geoips
+    pip install -e "$GEOIPS_PACKAGES_DIR/geoips[doc,lint,test,debug]"
+
+See the [ADDING FUNCTIONALITY] page for more details on how to contribute to GeoIPS.


If people are cloning the repo anyway, why not just do the editable install and leave it at that?

Using -e means the system points to the current folder, whereas usually it's copied elsewhere. So -e means if you delete the clone, your install will break.... Not sure if that's important, so I can just leave it as -e by default for now

Changed to -e by default

mindyls · 2024-07-22T16:16:43Z

docs/source/new-docs/getting-started/installing/index.rst

+Docker
+------
+
+We provide a working Dockerfile that can be used to run GeoIPS in a container.
+
+The Dockerfile can be built into a Docker image by cloning the GeoIPS repository and
+running ``docker build``. For example:
+
+.. code:: bash
+
+    git clone https://github.com/NRLMMD-GEOIPS/geoips.git geoips
+    cd geoips
+    docker build -t geoips .
+
+The Docker image can be run with the following command:
+
+.. code:: bash
+
+    docker run -it geoips   # Run the container in interactive mode
+
+Right now, the Docker image is only used for development and testing.
+
+We suggest mounting in a data directory so your containers don't get too large.
+
+If you are interested in using the Docker build
+for production or plugin development, please reach
+out to us via our contact page [CONTACT PAGE] or create an issue on GitHub [ISSUE PAGE].


I think this should be in a separate document. I think this installation should 100% only be for the "standard" conda based install.

I moved non-conda portions to a separate document

mindyls · 2024-07-22T16:26:56Z

docs/source/new-docs/getting-started/installing/index.rst

+.. code:: bash
+
+    # Ensure geoips python environment enabled before installing geoips
+    pip install "$GEOIPS_PACKAGES_DIR/geoips"


I would just take this out as well - just do the full editable install that is required for the tests to pass.

I changed it to -e. Is a full install required for tests?

Depends how we want people to test geoips_clavrx and data_fusion

mindyls · 2024-07-22T16:30:08Z

docs/source/new-docs/getting-started/installing/index.rst

+.. code:: bash
+
+    # Ensure geoips python environment enabled before installing geoips
+    pip install "$GEOIPS_PACKAGES_DIR/geoips[doc,lint,test,debug]"


I would just go with pip install -e here, and take out the next section.

I changed it to -e and moved the development section to a different document

mindyls · 2024-07-22T16:31:27Z

docs/source/new-docs/getting-started/installing/index.rst

+.. note::
+
+    If you would like to run plugins that require fortran, you will need to install ``gfortran``.
+


People will likely not know this in advance, so it may just confuse matters whether thye need it or not.

Removed, added to default install command

mindyls · 2024-07-22T18:46:17Z

docs/source/new-docs/getting-started/installing/basic-env-setup.rst

+.. code:: bash
+
+    # Ensure geoips python environment enabled before installing geoips
+    pip install -e "$GEOIPS_PACKAGES_DIR/geoips[test]"
+
+If you want to install GeoIPS with all optional dependencies, you can use:
+
+.. code:: bash
+
+    # Ensure geoips python environment enabled before installing geoips
+    pip install "$GEOIPS_PACKAGES_DIR/geoips[doc,lint,test,debug]"


I would ONLY include

pip install -e "$GEOIPS_PACKAGES_DIR/geoips[doc,lint,test,debug]"

Right now the tests will fail if doc,lint,test,debug are not all installed.

Just have one geoips installation method.

I'll add -e to the second line - is there any reason to install doc, lint, and debug? See above - my tests seem to be passing without them

mindyls · 2024-07-22T18:46:37Z

docs/source/new-docs/getting-started/installing/basic-env-setup.rst

+    # Ensure geoips python environment enabled before installing geoips
+    pip install "$GEOIPS_PACKAGES_DIR/geoips[doc,lint,test,debug]"
+
+The optional dependencies are:


I would make a note here:

"These optional dependencies are all required for tests to fully pass"

See my comments above 🙂

mindyls · 2024-07-22T18:47:46Z

docs/source/new-docs/getting-started/installing/install-geoips.rst

+Install GeoIPS
+--------------
+
+We can use ``pip`` to install all GeoIPS Python dependencies, and GeoIPS itself.
+
+First, clone the GeoIPS git repository:
+
+.. code:: bash
+
+    git clone https://github.com/NRLMMD-GeoIPS/geoips.git $GEOIPS_PACKAGES_DIR/geoips
+
+.. code:: bash
+
+    # Ensure geoips python environment enabled before installing geoips
+    pip install "$GEOIPS_PACKAGES_DIR/geoips"
+
+If you want to install GeoIPS with all optional dependencies, you can use:
+
+.. code:: bash
+
+    # Ensure geoips python environment enabled before installing geoips
+    pip install "$GEOIPS_PACKAGES_DIR/geoips[doc,lint,test,debug]"
+
+The optional dependencies are:
+
+- ``doc``: for building the documentation with Sphinx
+  (the documentation is also available online at
+  https://nrlmmd-geoips.github.io/geoips/)
+- ``lint``: for linting the code (useful for developers)
+- ``test``: for running the tests
+- ``debug``: for debugging the code with IPython/jupyter


What is the intention for this section of the installation?

The intention is to - later - embed it in the advanced and the basic installation instructions

mindyls · 2024-07-22T18:48:11Z

docs/source/new-docs/getting-started/best-practices/testing.rst

+:orphan:
+
+Testing
+=======
+
+.. note::
+
+    Setting up WSL through windows is the easiest method for running GeoIPS on
+    a Windows system. To do so, follow directions on
+    `Microsoft WSL Install <https://learn.microsoft.com/en-us/windows/wsl/install>`_.
+    This requires administrator privileges.


What is the intention for this section of the getting started?

The intention is to add more information about testing geoips in the future, but that's not needed here

mindyls · 2024-07-22T18:48:33Z

docs/source/new-docs/getting-started/installing/advanced-env-setup.rst

+Installing
+==========
+
+.. note::
+
+    Setting up WSL through windows is the easiest method for running GeoIPS on
+    a Windows system. To do so, follow directions on
+    `Microsoft WSL Install <https://learn.microsoft.com/en-us/windows/wsl/install>`_.
+    This requires administrator privileges.
+
+For all installation methods, you need to setup environmental variables.
+
+The required environmental variables are:
+
+  -``GEOIPS_PACKAGES_DIR`` is where GeoIPS looks for itself and its plugin packages
+  -``GEOIPS_TESTDATA_DIR`` is where GeoIPS looks to find its test datasets
+  -``GEOIPS_OUTDIRS`` defines where GeoIPS will write output data and imagery
+
+These are fine defaults for most users:
+
+.. code:: bash
+
+    export GEOIPS_PACKAGES_DIR=$HOME/geoips
+    export GEOIPS_TESTDATA_DIR=$GEOIPS_PACKAGES_DIR/test_data
+    export GEOIPS_OUTDIRS=$GEOIPS_PACKAGES_DIR/outdirs
+
+If desired, the GeoIPS environment variables can be added to your
+default shell configuration file (which is typically stored in


Are we going to link to this from anywhere, or just leave it orphaned for now, for later update / completion?

^ I think orphaned for now is fine

mindyls · 2024-07-22T18:49:20Z

docs/source/new-docs/getting-started/installing/basic-env-setup.rst

+ | # # # This source code is protected under the license referenced at
+ | # # # https://github.com/NRLMMD-GEOIPS.
+
+Installing


If this is going to be our "official" installatin method that we are sending out for people to use this week, then we will need to update the README.md to point here for installation.

Do we want to update this now?

This is what I tested, but I'm fine sending out the old instructions and waiting on this if we still have things to discuss

mindyls · 2024-07-23T03:48:47Z

docs/source/new-docs/getting-started/installing/basic-env-setup.rst

+
+        .. code:: bash
+
+            mamba create -y -n geoips -c conda-forge python=3.10 openblas git gfortran **pyhdf**


I might just comment this line so people don't do this by default. I don't know if it matters either way, but 99% sure people will just copy/paste their way through.

mindyls · 2024-07-23T03:51:39Z

docs/source/new-docs/getting-started/installing/basic-env-setup.rst

+  https://nrlmmd-geoips.github.io/geoips/)
+- ``lint``: for linting the code (useful for developers)
+- ``test``: for running the tests
+- ``debug``: for debugging the code with IPython/jupyter (also useful for developers)


I think it is worth having the base_install.sh and base_test.sh bundled as part of the basic installation - it is worth having immediate feedback that the install was successful. And it's really only 3 extra lines (base_install.sh, create_plugin_registries, and base_test.sh).

Oh - there is no create_plugin_registries included in this installation, so geoips is not fully functional at the end of this document.

biosafetylvl5 added 4 commits June 5, 2024 13:52

Working on installing

9c1bd78

finish conda install

dce320c

Update expert install, need feedback about whether or not other packa…

ba1a06b

…ges are also needed

Update installation docs

ed06cad

biosafetylvl5 linked an issue Jun 26, 2024 that may be closed by this pull request

Documentation: Environmental Setups for GeoIPS Installation #609

Open

8 tasks

biosafetylvl5 and others added 9 commits June 26, 2024 14:54

Merge remote-tracking branch 'origin/main' into documentation-geoips-…

516b271

…installation

Add WSL info

059f535

updates to install directions

b14d892

Add arm info

c89e13a

Update index.rst to fix install and add gb for depends (#664)

108ed4c

Merge branch 'main' into documentation-geoips-installation

0065696

Pinken

f8a9ae4

Merge branch 'main' into documentation-geoips-installation

df0c7db

Pinken

f15afb0

biosafetylvl5 marked this pull request as ready for review July 22, 2024 15:45

biosafetylvl5 self-assigned this Jul 22, 2024

biosafetylvl5 requested review from mindyls and jsolbrig July 22, 2024 15:45

mindyls reviewed Jul 22, 2024

View reviewed changes

biosafetylvl5 added 4 commits July 22, 2024 10:33

Remove extra information from install

0b11853

Change mamba to conda

30f39c4

Add editable mode, install test by default

469b585

Add fortran by default, remove extra info about it

4a5e1af

Pinken

cd718af

mindyls reviewed Jul 22, 2024

View reviewed changes

Change both install methods to -e

6e06c24

biosafetylvl5 added the documentation Improvements or additions to documentation label Jul 22, 2024

biosafetylvl5 and others added 2 commits July 22, 2024 14:08

Merge branch 'main' into documentation-geoips-installation

95c026b

Merge branch 'main' into documentation-geoips-installation

9c60e4d

mindyls reviewed Jul 23, 2024

View reviewed changes

Merge branch 'main' into documentation-geoips-installation

c0e51aa

biosafetylvl5 closed this Oct 4, 2024

biosafetylvl5 reopened this Oct 21, 2024

Merge branch 'main' into documentation-geoips-installation

8caf778

biosafetylvl5 marked this pull request as draft October 21, 2024 22:50

		.. note::

		If you would like to run plugins that require fortran, you will need to install ``gfortran``.


		.. code:: bash

		mamba create -y -n geoips -c conda-forge python=3.10 openblas git gfortran pyhdf

Documentation geoips installation #651

Are you sure you want to change the base?

Documentation geoips installation #651

Conversation

biosafetylvl5 commented Jun 26, 2024 • edited Loading

Related Issues

Testing Instructions

Summary

Output

Relevant XKCD: (Universal Install Script)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

biosafetylvl5 Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mindyls Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

biosafetylvl5 commented Jun 26, 2024 •

edited

Loading

biosafetylvl5 Jul 22, 2024 •

edited

Loading

mindyls Jul 23, 2024 •

edited

Loading