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

use PEP 420: implicit namespace package #990

Merged
merged 10 commits into from
Jul 14, 2022
86 changes: 35 additions & 51 deletions doc/api-docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,33 +12,26 @@
# All configuration values have a default; values that are commented out
# serve to show the default.

from datetime import datetime
import importlib
import itertools
import os
import pkgutil
import subprocess
import sys

from datetime import datetime
from sphinx.ext import apidoc
from unittest.mock import MagicMock

import natcap.invest
from sphinx.ext import apidoc
Copy link
Member

Choose a reason for hiding this comment

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

Could you just check these imports and make sure that we aren't importing anything we no longer need?

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right, a couple of them aren't needed any more


# If extensions (or modules to document with autodoc) are in another directory,
# add these directories to sys.path here. If the directory is relative to the
# documentation root, use os.path.abspath to make it absolute, like shown here.
DOCS_SOURCE_DIR = os.path.dirname(__file__)
INVEST_ROOT_DIR = os.path.join(DOCS_SOURCE_DIR, '..', '..')
INVEST_BUILD_DIR = os.path.join(INVEST_ROOT_DIR, 'build')
Copy link
Member

Choose a reason for hiding this comment

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

Are these used anywhere else now that we're properly defining INVEST_LIB_DIR? Might they be able to be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right, they can be removed


# get name of build/lib directory; this depends on the OS and python version
# e.g. lib.macosx-10.9-x86_64-3.8
for path in os.listdir(INVEST_BUILD_DIR):
if path.startswith('lib'):
INVEST_LIB_DIR = os.path.join(INVEST_BUILD_DIR, path)
break
else:
raise ValueError(f'lib directory not found in {INVEST_BUILD_DIR}')
# get the directory that the natcap package lives in
INVEST_LIB_DIR = os.path.dirname(os.path.dirname(natcap.invest.__file__))

# -- General configuration ------------------------------------------------

Expand Down Expand Up @@ -67,7 +60,6 @@
project = 'InVEST'
copyright = f'{datetime.now().year}, The Natural Capital Project'

import natcap.invest
# The full version, including alpha/beta/rc tags.
release = natcap.invest.__version__
version = release.split('+')[0]
Expand All @@ -77,7 +69,7 @@
#
# This is also used if you do content translation via gettext catalogs.
# Usually you set "language" from the command line for these cases.
language = None
language = 'en'
Copy link
Member Author

Choose a reason for hiding this comment

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

This was causing an error in the ReadTheDocs build, probably due to a sphinx update.


# List of patterns, relative to source directory, that match files and
# directories to ignore when looking for source files.
Expand Down Expand Up @@ -186,6 +178,7 @@ class MockQGroupBox:
'--templatedir', os.path.join(DOCS_SOURCE_DIR, 'templates'), # use custom templates
'--separate', # make a separate page for each module
'--no-toc', # table of contents page is redundant
'--implicit-namespaces',
INVEST_LIB_DIR,
'/*.so' # must be absolute path, see https://github.com/sphinx-doc/sphinx/issues/10200
])
Expand All @@ -202,29 +195,32 @@ class MockQGroupBox:

All InVEST models share a consistent python API:

1) The model has a function called ``execute`` that takes a single python
dict (``"args"``) as its argument.
2) This arguments dict contains an entry, ``'workspace_dir'``, which
points to the folder on disk where all files created by the model
should be saved.
- Every InVEST model has a corresponding module in the ``natcap.invest``
package
emlys marked this conversation as resolved.
Show resolved Hide resolved
- The model modules contain a function called ``execute``
- The ``execute`` function takes a single argument (``args``), a dictionary
that stores all data inputs and configuration options for the model
- This dictionary contains an entry, ``'workspace_dir'``, which is a path
to the folder where the model will create its output files
- Other required and optional entries in the dictionary are specific to
each model

Calling a model requires importing the model's execute function and then
calling the model with the correct parameters. For example, if you were
to call the Carbon Storage and Sequestration model, your script might
include
To run a model, import the model's ``execute`` function and then call it with
the correct parameters. For example, a script for the Carbon model might look
like

.. code-block:: python

import natcap.invest.carbon.carbon_combined
import natcap.invest.carbon
args = {
'workspace_dir': 'path/to/workspace'
# Other arguments, as needed for Carbon.
}

natcap.invest.carbon.carbon_combined.execute(args)
natcap.invest.carbon.execute(args)

For examples of scripts that could be created around a model run,
or multiple successive model runs, see :ref:`CreatingSamplePythonScripts`.
For examples of scripting a model run, or multiple successive model runs,
see :ref:`CreatingSamplePythonScripts`.


.. contents:: Available Models and Tools:
Expand All @@ -235,45 +231,33 @@ class MockQGroupBox:
EXCLUDED_MODULES = [
'_core', # anything ending in '_core'
'recmodel_server',
'recmodel_workspace_fetcher'
'recmodel_workspace_fetcher',
'natcap.invest.ui'
Copy link
Member

Choose a reason for hiding this comment

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

Based on the RTD-rendered docs (https://invest--990.org.readthedocs.build/en/990/api.html for this PR) it looks like maybe this exclusion isn't working. Maybe it never worked? In any case, it would probably be nice to exclude packages, or else just have everything in there since it is in the source code.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is working as expected - it's excluding those from the model entrypoints file models.rst. But they're still in the API docs.
The list of excluded modules was unnecessary, though. I refactored so that any module with an ARGS_SPEC is included in models.rst.

]
MODEL_ENTRYPOINTS_FILE = os.path.join(DOCS_SOURCE_DIR, 'models.rst')

# Find all importable modules with an execute function
# write out to a file models.rst in the source directory
all_modules = {}
for _loader, name, _is_pkg in itertools.chain(
pkgutil.walk_packages(path=[INVEST_LIB_DIR]), # catch packages
pkgutil.iter_modules(path=[INVEST_LIB_DIR])): # catch modules

if (any([name.endswith(x) for x in EXCLUDED_MODULES]) or
name.startswith('natcap.invest.ui')):
for _, name, _ in pkgutil.walk_packages(path=[INVEST_LIB_DIR],
prefix='natcap.'):
if any(x in name for x in EXCLUDED_MODULES):
continue

try:
module = importlib.import_module(name)
except Exception as ex:
print(ex)
continue
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed this because I think we want to raise an error if a module doesn't import.

Copy link
Member

Choose a reason for hiding this comment

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

I agree, better to fail loudly.

module = importlib.import_module(name)

if not hasattr(module, 'execute'):
continue

try:
module_title = module.execute.__doc__.strip().split('\n')[0]
if module_title.endswith('.'):
module_title = module_title[:-1]
except AttributeError:
module_title = None
all_modules[name] = module_title
# all modules with an execute function should have an ARGS_SPEC
model_title = module.ARGS_SPEC['model_name']
all_modules[model_title] = name
Copy link
Member

Choose a reason for hiding this comment

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

This is so much better than our prior reliance on module-level docstrings!


# Write sphinx autodoc function for each entrypoint
with open(MODEL_ENTRYPOINTS_FILE, 'w') as models_rst:
models_rst.write(MODEL_RST_TEMPLATE)
for name, module_title in sorted(all_modules.items(), key=lambda x: x[1]):
underline = ''.join(['=']*len(module_title))
for model_title, name in sorted(all_modules.items()):
underline = ''.join(['=']*len(model_title))
models_rst.write(
f'{module_title}\n'
f'{model_title}\n'
f'{underline}\n'
f'.. autofunction:: {name}.execute\n'
' :noindex:\n\n')
26 changes: 12 additions & 14 deletions requirements-docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,34 +12,32 @@ channels:
dependencies:
- chardet>=3.0.4
- Cython
- GDAL>=3.1.2,!=3.3.0
- GDAL>=3.1.2,!=3.3.0,<3.4.1
- Flask
- flask_cors
- numpy>=1.11.0,!=1.16.0
- pandas>=1.0,<1.2.0
- pandas>=1.2.1
- pip
- psutil>=5.6.6
- qtpy>1.3
- requests
- Rtree==0.9.4
- Rtree>=0.8.2,!=0.9.1
- setuptools>=8.0
- Shapely>=1.7.1,<2.0.0
- sphinx==3.3.1
- setuptools_scm>=6.4.0
- shapely>=1.7.1,<1.8.2
- sphinx
- sphinx-intl
- scipy>=1.6.0
- taskgraph[niced_processes]>=0.10.2
- taskgraph[niced_processes]>=0.11.0
- virtualenv>=12.0.1
- wheel>=0.27.0
- xlrd>=1.2.0
- jinja2<3.1
- pint
- xlwt
- pip:
- pygeoprocessing>=2.3.2 # pip-only
- PyInstaller==3.5 # pip-only
- Pyro4==4.77 # pip-only
- PySide2!=5.15.0 # pip-only
- qtawesome # pip-only
- setuptools_scm>=6.4.0
- pygeoprocessing>=2.3.2
- PyInstaller>=4.10
- Pyro4==4.77
- PySide2!=5.15.0
- qtawesome
- sphinx-rtd-theme
- sphinx-reredirects
16 changes: 2 additions & 14 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

import Cython.Build
import numpy
from setuptools import setup
from setuptools import find_namespace_packages, setup
from setuptools.command.build_py import build_py as _build_py
from setuptools.extension import Extension

Expand Down Expand Up @@ -69,19 +69,7 @@ def run(self):
maintainer='James Douglass',
maintainer_email='jdouglass@stanford.edu',
url='http://github.com/natcap/invest',
namespace_packages=['natcap'],
packages=[
'natcap',
'natcap.invest',
'natcap.invest.coastal_blue_carbon',
'natcap.invest.delineateit',
'natcap.invest.ui',
'natcap.invest.ndr',
'natcap.invest.sdr',
'natcap.invest.recreation',
'natcap.invest.scenic_quality',
'natcap.invest.seasonal_water_yield',
],
packages=find_namespace_packages('src'),
Copy link
Member

Choose a reason for hiding this comment

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

Great! It was not immediately clear to me from this function name that normal packages within natcap.invest would also be captured, but that does seem to be the behavior.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it's confusing! pypa/setuptools#3427

package_dir={
'natcap': 'src/natcap'
},
Expand Down
4 changes: 0 additions & 4 deletions src/natcap/__init__.py

This file was deleted.