Skip to content

Comments

Restrict mkdocstrings and mkdocstrings-python version range#155

Merged
Baharis merged 3 commits intoinstamatic-dev:mainfrom
Baharis:fix_docs
Jan 15, 2026
Merged

Restrict mkdocstrings and mkdocstrings-python version range#155
Baharis merged 3 commits intoinstamatic-dev:mainfrom
Baharis:fix_docs

Conversation

@Baharis
Copy link
Member

@Baharis Baharis commented Jan 14, 2026

After merging #154 yesterday I noticed that the docs are not being compiled properly. It seems that recently both mkdocstrings and mkdocstrings-python increased their major version, and the new combination fails with the following exception. I configured read the docs to trigger documentation build on every PR and restricted the version range of both packages to address the issue:

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/home/docs/checkouts/readthedocs.org/user_builds/instamatic/envs/latest/lib/python3.11/site-packages/mkdocs/__main__.py", line 370, in <module>
    cli()
  File "/home/docs/checkouts/readthedocs.org/user_builds/instamatic/envs/latest/lib/python3.11/site-packages/click/core.py", line 1485, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/docs/checkouts/readthedocs.org/user_builds/instamatic/envs/latest/lib/python3.11/site-packages/click/core.py", line 1406, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/home/docs/checkouts/readthedocs.org/user_builds/instamatic/envs/latest/lib/python3.11/site-packages/click/core.py", line 1873, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/docs/checkouts/readthedocs.org/user_builds/instamatic/envs/latest/lib/python3.11/site-packages/click/core.py", line 1269, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/docs/checkouts/readthedocs.org/user_builds/instamatic/envs/latest/lib/python3.11/site-packages/click/core.py", line 824, in invoke
    return callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/docs/checkouts/readthedocs.org/user_builds/instamatic/envs/latest/lib/python3.11/site-packages/mkdocs/__main__.py", line 288, in build_command
    build.build(cfg, dirty=not clean)
  File "/home/docs/checkouts/readthedocs.org/user_builds/instamatic/envs/latest/lib/python3.11/site-packages/mkdocs/commands/build.py", line 265, in build
    config = config.plugins.on_config(config)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/docs/checkouts/readthedocs.org/user_builds/instamatic/envs/latest/lib/python3.11/site-packages/mkdocs/plugins.py", line 587, in on_config
    return self.run_event('config', config)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/docs/checkouts/readthedocs.org/user_builds/instamatic/envs/latest/lib/python3.11/site-packages/mkdocs/plugins.py", line 566, in run_event
    result = method(item, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/docs/checkouts/readthedocs.org/user_builds/instamatic/envs/latest/lib/python3.11/site-packages/mkdocstrings/_internal/plugin.py", line 153, in on_config
    handlers._download_inventories()
  File "/home/docs/checkouts/readthedocs.org/user_builds/instamatic/envs/latest/lib/python3.11/site-packages/mkdocstrings/_internal/handlers/base.py", line 600, in _download_inventories
    handler = self.get_handler(handler_name)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/docs/checkouts/readthedocs.org/user_builds/instamatic/envs/latest/lib/python3.11/site-packages/mkdocstrings/_internal/handlers/base.py", line 581, in get_handler
    self._handlers[name] = module.get_handler(
                           ^^^^^^^^^^^^^^^^^^^
  File "/home/docs/checkouts/readthedocs.org/user_builds/instamatic/envs/latest/lib/python3.11/site-packages/mkdocstrings_handlers/python/_internal/handler.py", line 403, in get_handler
    config=PythonConfig.from_data(**handler_config),
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/docs/checkouts/readthedocs.org/user_builds/instamatic/envs/latest/lib/python3.11/site-packages/mkdocstrings_handlers/python/_internal/config.py", line 1142, in from_data
    return cls(**cls.coerce(**data))
           ^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: PythonConfig.__init__() got an unexpected keyword argument 'import'

@Baharis
Copy link
Member Author

Baharis commented Jan 14, 2026

I confirm that with mkdocstrings=0.30.1 and mkdocstrings-python=1.19.0, the documentation is now properly built again. Whether building the documentation succeeds or not can be checked by visiting https://app.readthedocs.org/projects/instamatic/builds/, or by going to https://instamatic.readthedocs.io/, extending the menu next to "latest" version annotation, and selecting "Builds". @stefsmeets I do not have privileges to do this, but based on the docs if ReadTheDocs account got both repo:status and admin:repo_hook permissions, it should be able to show the status of docs building as one of the PR checks. I guess this PR is as good place to test this feature as it gets.

Copy link
Member

@stefsmeets stefsmeets left a comment

Choose a reason for hiding this comment

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

Good catch, mkdocs isn't being developed anymore and the developers of mkdocs-material and mkdocstrings started a fork of mkdocs: https://github.com/zensical/zensical

Long term (maybe not that long), once zensical matures it should be relatively straightforward to migrate. Although there is nothing wrong with mkdocs =)

Seems like this particular issues is caused by these lines:

instamatic/mkdocs.yml

Lines 72 to 76 in 941750b

import:
- https://docs.python.org/3/objects.inv
- https://numpy.org/doc/stable/objects.inv
- https://docs.scipy.org/doc/scipy/objects.inv
- https://pandas.pydata.org/docs/objects.inv

Where 'import' should be changed to 'inventories', could you see if that fixes the issue?

@stefsmeets
Copy link
Member

stefsmeets commented Jan 15, 2026

@stefsmeets I do not have privileges to do this, but based on the docs if ReadTheDocs account got both repo:status and admin:repo_hook permissions, it should be able to show the status of docs building as one of the PR checks.

I just looked and you have maintainer rights on readthedocs. Your PR is from your personal account, that's why it probably does not show up.

I usually just enable a version (linked to the branch) when I'm debugging a rtd build in a PR. The new rtd dashboard is a bit confusing to me, but you can just click a branch and tick active+hidden to enable it. It must be a branch on the instamatic repo though.

I avoid building the docs in the PR as a check, because it's a waste of resources in 9/10 cases and if it catches anything it's usually not related to the PR.

@Baharis
Copy link
Member Author

Baharis commented Jan 15, 2026

@stefsmeets I do have maintainer permissions on readthedocs, that's why I could start compiling docs for PR in the first place, but from what I read, adding a feedback into the PR reviews on GitHub requires some administrative work on GitHub.

It is reasonable to check docs status using separate docs version, but as you mentioned the branch needs to be on instamatic-dev/, which effectively prevents anyone other than you from using this feature. Meanwhile anyone can create a PR, and what is the point of making sure everyone documents their features if we don't know whether the documentation will be updated.

As you mentioned, in 9/10 cases PRs do not introduce any significant changes to documentation, but like in the recent case sometimes the documentation stops compiling not due to changes to documentation, but due to issues with back end. We spent 2 months not knowing the docs did not update, and a few 30-second compilations on each PR would show that to us clearly. I would argue a proper integration and adding a big green light @ PR signaling docs are fine is anything but a waste of resources.

@stefsmeets
Copy link
Member

Should add a status badge for readthedocs to the readme:

@Baharis
Copy link
Member Author

Baharis commented Jan 15, 2026

After replacing import with inventories, the documentation now properly builds using the pew versions, so I removed the upper version limit. While at the subject, I would also like to suggest adding at least a stable version of documentation (tentatively activated, but fails due to issue above). Or even adding a version of documentation for each version of code. Ultimately how can people know what their version is capable of, compared to the current experimental i.e. latest?

@stefsmeets
Copy link
Member

which effectively prevents anyone other than you from using this feature. Meanwhile anyone can create a PR

Can you not push branches?

@Baharis
Copy link
Member Author

Baharis commented Jan 15, 2026

I can push branches to instamatic-dev/ using PRs, not sure if I can do it directly though - likely I can, but it feels like a bad practice to push a branch to another repo only to take a look if the documentation builds - aspecially if the alternative is an auto-build on PR.

One more argument concerning resources: the community version of Read the docs is free and they literally give you a built-in option to build on PR, I don't feel like we need to be super conscious about resource use here: https://docs.readthedocs.com/platform/latest/pull-requests.html.

@stefsmeets
Copy link
Member

stefsmeets commented Jan 15, 2026

Ah, I see. I always branch directly from the main repo. If you need it to work effectively then please do so.

I tend to reduce my CI usage where I can, mostly in attempt to reduce my environmental impact. One example is the test workflow that only runs when a PR is marked as 'ready for review' and aggressive caching where possible (I also don't like waiting 😅 ). No need to run the entire test suite for every commit in a PR. It kills me when I fix a small typo and the entire test suite runs for 5 python versions. I use it mostly to sign off on the final check before merging.

@Baharis
Copy link
Member Author

Baharis commented Jan 15, 2026

As mentioned, I myself apparently don't have enough permissions to add readthedocs "check" in GitHub. For the time being all I can do is enable or disable auto-docs on PR in readthedocs only. Having a "check" included with PR would require someone with GitHub admin privileges to add this. Since I do not want to force anything upon yourself, and made this PR primarily to fix the docs, I will merge this request and leave any potential changes to the checks for the next one.

@Baharis Baharis merged commit b20f070 into instamatic-dev:main Jan 15, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants