Skip to content

Conversation

@ginaarnau
Copy link
Contributor

No description provided.

Copy link
Member

@wkearn wkearn left a comment

Choose a reason for hiding this comment

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

Looks pretty good. There is one issue with the load_opentopography test detailed in a comment there.

Can you also add the doctest extension to docs/conf.py? This will let us use doctest in our documentation build process. Add a line to the extensions list at the top of the file:

@@ -9,6 +9,7 @@ import topotoolbox
 extensions = [
     'sphinx.ext.autodoc',
     'sphinx.ext.autosummary',
+    'sphinx.ext.doctest',
     'sphinx.ext.napoleon',
     'sphinx.ext.viewcode',
     'sphinx.ext.todo',

Comment on lines -463 to +486
>>> dem = topotoolbox.load_open_topography(south=50, north=50.1, west=14.35,
east=14.6, dem_type="SRTMGL3", api_key="demoapikeyot2022")
>>> dem = dem.reproject(rasterio.CRS.from_epsg(32633), resolution=90)
>>> im = dem.plot(cmap="terrain")
>>> import topotoolbox
>>> import matplotlib.pyplot as plt
>>> from rasterio import CRS
>>> dem = topotoolbox.load_dem('kunashiri')
>>> dem = dem.reproject(CRS.from_epsg(32655), resolution=30)
>>> _= dem.plot(cmap="terrain")
Copy link
Member

Choose a reason for hiding this comment

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

This example is supposed to demonstrate the load_opentopography function, so we can't replace it with loading the geographic DEM like in the reproject example. It is better to leave it as

>>> import topotoolbox
>>> import matplotlib.pyplot as plt
>>> from rasterio import CRS
>>> dem = topotoolbox.load_dem('kunashiri')
>>> dem = dem.reproject(CRS.from_epsg(32655), resolution=30)
>>> dem = topotoolbox.load_opentopography(south=50, north=50.1, west=14.35,
...     east=14.6, dem_type="SRTMGL3")
>>> dem = dem.reproject(CRS.from_epsg(32633), resolution=90)
>>> _= dem.plot(cmap="terrain")
>>> plt.show()

Also note that I got rid of the demo API key here. We shouldn't use that for anything that we are going to run regularly. If we run the test on GitHub Actions, we can use the OpenTopography API key that we have stored in the repository.

If you run into problems with this test, either because you don't have an API key or because you have problems accessing the DEM over the network, you can skip this example by adding # doctest: +SKIP to each line that you don't want to run:

>>> import topotoolbox
>>> import matplotlib.pyplot as plt
>>> from rasterio import CRS
>>> dem = topotoolbox.load_dem('kunashiri')
>>> dem = dem.reproject(CRS.from_epsg(32655), resolution=30)
>>> dem = topotoolbox.load_opentopography(south=50, north=50.1, west=14.35,
...     east=14.6, dem_type="SRTMGL3") # doctest: +SKIP
>>> dem = dem.reproject(CRS.from_epsg(32633), resolution=90) # doctest: +SKIP
>>> _= dem.plot(cmap="terrain") # doctest: +SKIP
>>> plt.show()

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 I broke the code. I simply made those changes, but when I try to run sphinx doctest, these errors occur:

C:\Users\hp\anaconda3\envs\TopoToolbox\lib\site-packages\topotoolbox\stream_object.py:docstring of topotoolbox.stream_object.StreamObject.crs:11: WARNING: Title underline too short.

Parameters:
---------- [docutils]
C:\Users\hp\anaconda3\envs\TopoToolbox\lib\site-packages\topotoolbox\stream_object.py:docstring of topotoolbox.stream_object.StreamObject.crs:14: ERROR: Unexpected indentation. [docutils]
C:\Users\hp\anaconda3\envs\TopoToolbox\lib\site-packages\topotoolbox\stream_object.py:docstring of topotoolbox.stream_object.StreamObject.crs:15: WARNING: Block quote ends without a blank line; unexpected unindent. [docutils]
C:\Users\hp\anaconda3\envs\TopoToolbox\lib\site-packages\topotoolbox\stream_object.py:docstring of topotoolbox.stream_object.StreamObject.crslin:10: WARNING: Title underline too short.

Parameters:
---------- [docutils]
C:\Users\hp\anaconda3\envs\TopoToolbox\lib\site-packages\topotoolbox\stream_object.py:docstring of topotoolbox.stream_object.StreamObject.crslin:13: ERROR: Unexpected indentation. [docutils]
C:\Users\hp\anaconda3\envs\TopoToolbox\lib\site-packages\topotoolbox\stream_object.py:docstring of topotoolbox.stream_object.StreamObject.crslin:14: WARNING: Block quote ends without a blank line; unexpected unindent. [docutils]

Exception occurred:
File "C:\Users\hp\anaconda3\envs\TopoToolbox\lib\os.py", line 225, in makedirs
mkdir(name, mode)
FileNotFoundError: [WinError 206] The filename or extension is too long: 'C:\Users\hp\Desktop\Princeton\Germany\Internship Project\pytopotoolbox\docs\build\doctest\.doctrees\nbsphinx\build/doctest/.doctrees/nbsphinx/build/doctest/.doctrees/nbsphinx/build/doctest/.doctrees/nbsphinx/build/doctest/.doctrees/nbsphinx/build/doctest'
The full traceback has been saved in C:\Users\hp\AppData\Local\Temp\sphinx-err-cwj3m01_.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at https://github.com/sphinx-doc/sphinx/issues. Thanks!

I see most of the suff has to do with "crslin" in StreamObject, but I haven't changed anything from there since I last ran the tests, so I don't know what is happening (I did change crslin, but in the branch I am currently wokring at these changes haven't been updated yet.)

Copy link
Member

Choose a reason for hiding this comment

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

I think the issues with crs and crslin are mostly not fatal errors related to the docstring format.

The real error is the filename or extension is too long error. It looks like sphinx is running recursively, generating a directory inside a directory and then trying to rerun from there, so you eventually get a filename that is too long.

How are you running sphinx? I think when we added the sphinx.ext.doctest extension to conf.py it might have changed how you need to run things. With the extension, running

make doctest

from a terminal (not a Python interpreter but the bash or Powershell terminal where you can run git commands) should work.

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 am running sphinx using this command: sphinx-build -b doctest docs docs/build/doctest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I run it like that I get:

The term 'make' is not recognized as the name of a cmdlet, function, script file, or operable prog
ram. Check the spelling of the name, or if a path was included, verify that the path is correct and try a
gain.

I did add doctest in conf, but I don't think I committed that change yet. I was waiting to commit it wiht these updated changes

Copy link
Member

Choose a reason for hiding this comment

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

You probably don't have make installed. That's all right. It is actually just running the same command you are using, so I don't think that's the problem, necessarily.

Copy link
Member

Choose a reason for hiding this comment

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

What's happening is that you are building the docs into the docs/build/doctest directory, and then the next time you run your sphinx-build command, it finds a bunch of doctests in there and tries to run them again. This happens until your filepaths get so big that they hit Windows' filepath length limit.

To fix this, delete your docs/build directory, then next time you run Sphinx, run this instead:

sphinx-build -b doctest docs docs/_build/doctest

Note the underscore in _build. We have Sphinx set up to ignore files in the _build directory, but you were calling it build instead, which caused this recursion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants