-
Notifications
You must be signed in to change notification settings - Fork 64
Fix API doc build #556
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
Fix API doc build #556
Conversation
|
Integration tests report: appsharing.space |
Unclear why removed in geojupyter#412
Setting up the environment should probably be decoupled from the build helper script.
|
Some context: https://github.com/geojupyter/jupytergis/pull/412/files#r1942477561 cc @martinRenou @davidbrochart Could you all please provide a summary of that discussion thread? I'm struggling to understand why the lines which installed the built JupyterGIS wheels into the docs environment were removed; that turned out to be the root cause of the missing API docs. I'm also curious about what we should do about |
This actually worked locally 🙏
``` /home/docs/micromamba/envs/jupytergis-docs/lib/python3.12/site-packages/jupytergis_lab/notebook/gis_document.py:docstring of jupytergis_lab.notebook.gis_document.GISDocument:5: WARNING: Field list ends without a blank line; unexpected unindent. [docutils] ```
|
Whew, finally got it! To get it passing without any warnings I needed to change some docstrings and type annotations. |
| feature: str | None = None, | ||
| operator: str | None = None, | ||
| value: Union[str, number, float] | None = None, | ||
| value: Union[str, int, float] | None = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switching between typescript and python is a big oof on my brain 🤕
| :param normalize: Select whether to normalize values between 0..1, if false than min/max have no effect, defaults to True | ||
| :param wrapX: Render tiles beyond the tile grid extent, defaults to False | ||
| :param opacity: The opacity, between 0 and 1, defaults to 1.0 | ||
| :param color_expr: The style expression used to style the layer, defaults to None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Repeating the types in the docstring is not necessary, the documentation generator will find the annotations in the function signature!
| """Build all JupyterGIS packages. | ||
| IMPORTANT: Requires dependencies in requirements-build.txt | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I felt we should decouple dependency install from building.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed! Thanks
The removal of jupyterlite-qgis was to not have the extension installed for jupyterlite. But it may be harmless as I see the jupyterlite deployment working well.
Description
The API docs are currently not building because of a failure to import the documented module. Worse, this only displays as a warning, so this has been happening silently for some time (since #412). The content in the HTML has been missing 😱
--fail-on-warning: Warnings fail the build--keep-going: Don't stop building when warnings occur; complete the build, show all warnings, and fail at the end.--nitpicky: Fail on bad internal linksChecklist
Resolves #XXX.Failing lint checks can be resolved with:
pre-commit run --all-filesjlpm run lint📚 Documentation preview: https://jupytergis--556.org.readthedocs.build/en/556/
💡 JupyterLite preview: https://jupytergis--556.org.readthedocs.build/en/556/lite