Skip to content

feat: teach docs about object store#7169

Merged
danking merged 6 commits intodevelopfrom
dk/object-store-docs
Mar 26, 2026
Merged

feat: teach docs about object store#7169
danking merged 6 commits intodevelopfrom
dk/object-store-docs

Conversation

@danking
Copy link
Contributor

@danking danking commented Mar 25, 2026

A user asked a question about object store today in Slack. When trying to answer that question, I realized our object store docs are garbage because they're all stored in pyi files which are not used by Sphinx.

Hopefully you will excuse the hack I used here. I copied those docstrings over into real py files and sub-classed the actual native classes of interest. These non-native classes get documented as you expect and delegate their methods to their (native) super-classes.

I forget why I updated Sphinx but it was due to something not rendering properly on our old version of Sphinx.

A user asked a question about object store today in Slack. When trying to answer that question, I
realized our object store docs are garbage because they're all stored in pyi files which are not
used by Sphinx.

Hopefully you will excuse the hack I used here. I copied those docstrings over into real py files
and sub-classed the actual native classes of interest. These non-native classes get documented as
you expect and delegate their methods to their (native) super-classes.

Signed-off-by: Daniel King <dan@spiraldb.com>
@danking danking requested review from a10y and gatesn March 25, 2026 23:14
@danking danking marked this pull request as ready for review March 25, 2026 23:14
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 25, 2026

Merging this PR will not alter performance

✅ 1106 untouched benchmarks
⏩ 1522 skipped benchmarks1


Comparing dk/object-store-docs (e30a405) with develop (c8eae59)

Open in CodSpeed

Footnotes

  1. 1522 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@a10y
Copy link
Contributor

a10y commented Mar 26, 2026

Copy link
Contributor

@a10y a10y left a comment

Choose a reason for hiding this comment

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

approving b/c i am a dolt at python tooling and if this works I'm happy, I'm just wondering why we can't do what obstore does

@danking
Copy link
Contributor Author

danking commented Mar 26, 2026

Ah, they have a clever alternative to what I did.

I moved (really: copied) the docs from the pyi files to py files and placed the docs on shim sub-classes. I then used Sphinx to document those sub-classes.

Obstore documents the native library in pyi files, then creates shim sub-classes in normal Python but they do not define any methods. They request mdbook to show inherited members.

Let me see if I can get that to work.

@danking
Copy link
Contributor Author

danking commented Mar 26, 2026

OK, I don't think it is possible to make this work for us because Sphinx walks the module hierarchy and we create a partly broken module hierarchy.

This PR introduced functionality to read docs from pyi files for native modules. The code that looks for pyi files assumes that modules have a ModuleSpec at __spec__. Our native library is a real module:

In [4]: find_spec("vortex._lib")
Out[4]: ModuleSpec(name='vortex._lib', loader=<_frozen_importlib_external.ExtensionFileLoader object at 0x10923fb90>, origin='/Users/danielking/projects/spiraldb/.venv/lib/python3.13/site-packages/vortex/_lib.abi3.so')

but the modules within it are not real modules. In particular, when Python imports files, it configures the module spec properly. Even if we wanted to cheat and create a ModuleSpec ourselves, there is no valid origin for our module. It is defined _lib.ab3.so but then the path to the pyi files would be _lib.pyi. All our modules would share the same pyi file.

In [5]: find_spec("vortex._lib.store")
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[5], line 1
----> 1 find_spec("vortex._lib.store")

File <frozen importlib.util>:111, in find_spec(name, package)

ValueError: vortex._lib.store.__spec__ is None

I suspect that, in obstore's case, MdBook is a bit more fast-and-loose with module discovery.

@danking
Copy link
Contributor Author

danking commented Mar 26, 2026

FWIW, I think the approved way to do this would be to have each module be a separate native object rather than trying to cook up a module hierarchy ourselves in the init method for our _lib native module.

@danking danking added the changelog/docs A docs change label Mar 26, 2026
@danking danking enabled auto-merge (squash) March 26, 2026 15:25
@gatesn
Copy link
Contributor

gatesn commented Mar 26, 2026

Having separate modules is a very different thing in Python land. It's typically not done afaict? But the thing we do with sys modules is also very weird...

@danking
Copy link
Contributor Author

danking commented Mar 26, 2026

Agreed, I think most folks just dump everything into the single native module and then cook up the hierarchy in Python-land. If we want our pyi files to play well with native modules, then we need 1:1 module to native library.

danking added 5 commits March 26, 2026 11:50
Signed-off-by: Daniel King <dan@spiraldb.com>
Signed-off-by: Daniel King <dan@spiraldb.com>
Signed-off-by: Daniel King <dan@spiraldb.com>
Signed-off-by: Daniel King <dan@spiraldb.com>
Signed-off-by: Daniel King <dan@spiraldb.com>
@danking danking force-pushed the dk/object-store-docs branch from 1638458 to e30a405 Compare March 26, 2026 15:50
@danking danking merged commit be4761a into develop Mar 26, 2026
62 checks passed
@danking danking deleted the dk/object-store-docs branch March 26, 2026 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/docs A docs change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants