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

Griffe with scikit-build-core does not work in editable mode #154

Closed
JeremyBYU opened this issue May 3, 2023 · 17 comments
Closed

Griffe with scikit-build-core does not work in editable mode #154

JeremyBYU opened this issue May 3, 2023 · 17 comments
Assignees

Comments

@JeremyBYU
Copy link

Describe the bug
Griffe is unable to find modules for a package built in editable mode when using scikit-build-core (pip install -e .) . If you build the module and install regularly (pip install .) everything works as expected. I have created a repository (with instructions) that replicates this issue: https://github.com/JeremyBYU/griffe-scikit-build-core-bug

@pawamoy
Copy link
Member

pawamoy commented May 3, 2023

Hey, thanks for the report!

I'm getting a 404, is the repo public?

@JeremyBYU
Copy link
Author

JeremyBYU commented May 3, 2023 via email

@pawamoy pawamoy self-assigned this May 8, 2023
pawamoy added a commit that referenced this issue May 10, 2023
…dules

We now support `scikit-build-core` editable modules as well.

Issue #154: #154
@pawamoy
Copy link
Member

pawamoy commented May 10, 2023

Fixed, will push a release 🙂

@pawamoy pawamoy closed this as completed May 10, 2023
@JeremyBYU
Copy link
Author

Thanks will try it out soon!

@JeremyBYU
Copy link
Author

This does not seem to fully fix the issue for me. I updated the bug repository to generate documentation using mkdocs and instructions to reproduce.

@pawamoy
Copy link
Member

pawamoy commented May 12, 2023

Thanks for the update. Yeah the {'griffedemo._core': 'griffedemo\\_core.cp310-win_amd64.pyd'} part wasn't there before. I know nothing about scikit-build-core so I don't understand why you get a .pyd file in a different place than the rest.

Based on the other paths, your project root is C:\\Users\\Jerem\\Documents\\Springfield\\Research\\griffe-scikit-build-core-bug right? Then what does griffedemo\\_core.cp310-win_amd64.pyd refer to? Is it supposed to be C:\\Users\\Jerem\\Documents\\Springfield\\Research\\griffe-scikit-build-core-bug\\src\\griffedemo\\_core.cp310-win_amd64.pyd? If so, the src part is missing and that will be hard to infer that... But even if it was, the first path would have been enough, and Griffe would have found all modules under src/griffedemo, including the .pyd file.

Could you post the resulting file system tree, so that I can see where the .pyd file is located? With a command like tree if you have it. Or a recursive ls. Thanks 🙂

@JeremyBYU
Copy link
Author

Yes you are correct. In my haste to put out this bug report I made a few blunders. The main one was here where I accidentally left this as cmake_example and installed it directly in the site-packages folder (thats the period in the end). I changed it so that the module is now called _core (a common pattern for pybind11 modules) and correctly installs in the package name folder griffedemo. I think those were the only major changes that I made when it comes to installation paths and such.

Aside: Usually the purepython package (griffedemo in our case) released alongside the compiled module will import the _core and expose the member classes/functions to library consumers.

Here are my directories after pip install -e .

griffe-scikit-build-core-bug
├─ src
│  ├─ main.cpp
│  └─ griffedemo
│     ├─ __init__.py
│     └─ purepython
│        └─ __init__.py
├─ README.md
├─ pyproject.toml
├─ mkdocs.yml
├─ docs
│  ├─ purepython.md
│  ├─ index.md
│  └─ compiled.md
└─ CMakeLists.txt

No compiled module inside here. The compiled module will actually be installed in the site-packages folder in the virtual environment

site_packages
├─ griffedemo
│     └─ _core.cp310-win_amd64.pyd
| ..... many packages
├─ _griffedemo_editable.pth
└─ _griffedemo_editable.py

Here are my directories after pip install .

griffe-scikit-build-core-bug
├─ src
│  ├─ main.cpp
│  └─ griffedemo
│     ├─ __init__.py
│     └─ purepython
│        └─ __init__.py
├─ README.md
├─ pyproject.toml
├─ mkdocs.yml
├─ docs
│  ├─ purepython.md
│  ├─ index.md
│  └─ compiled.md
└─ CMakeLists.txt

Again, no compiled module inside here. The compiled module will actually be installed in the site-packages folder but so will be the pure python code.

site_packages
├─ griffedemo
│   ├─ __pycache__
│   │  └─ __init__.cpython-310.pyc
│   ├─ __init__.py
│   ├─ _core.cp310-win_amd64.pyd
│   └─ purepython
│      ├─ __pycache__
│      │  └─ __init__.cpython-310.pyc
│      └─ __init__.py

Hopefully the debug repo is in good shape now to help solve the problem.

@pawamoy
Copy link
Member

pawamoy commented May 12, 2023

Thanks, that's perfect. I was also able to use your reproduction repo locally (thanks a lot by the way, amazing report!!).

So, this is problematic. The editable install points at your src folder, which is handled by Griffe. But the compiled module is indeed installed in site-packages/griffedemo.

The issue is that src/griffedemo contains an __init__.py module, while site-packages/griffedemo does not. Both cannot usually co-exist (one is a namespace package, the other is not and takes precedence). I guess it works because of the dynamic meta path finder used by scikit-build-core.

#148 could probably help us here, but I don't know the implications yet. I strived to design Griffe as to not rely on sys.path or any execution of Python code. In this case "static" search is not enough.

I'll try to see how other frameworks that compile modules handle editable installs.

@pawamoy pawamoy reopened this May 12, 2023
@pawamoy
Copy link
Member

pawamoy commented May 12, 2023

Heh, anyway we could still somehow use griffedemo/_core.cpython-311-x86_64-linux-gnu.so (or Windows .pyd equivalent) in _griffedemo_editable.py to find it. I'll think of something 🙂

#148 is very promising but doesn't help at discovering all modules:

>>> _find_spec("griffedemo", None)
ModuleSpec(
    name='griffedemo',
    loader=<_frozen_importlib_external.SourceFileLoader object at 0x7fcc380b2f90>,
    origin='/media/data/dev/griffe-scikit-build-core-bug/src/griffedemo/__init__.py',
    submodule_search_locations=['/media/data/dev/griffe-scikit-build-core-bug/src/griffedemo']
)
# scanning griffe-scikit-build-core-bug/src/griffedemo will not give us the compiled module

@JeremyBYU
Copy link
Author

Thanks for looking into this. A final thing I may add is the possibility of using .pyi files to document the compiled module. The basic idea is that you provide the function signatures, typings, and docstrings in the pyi files (but no implementation). These pyi files live in the src tree as well, even in editable mode. They are also still correctly exposed in scikit-build-core (see this issue). The dynamic meta path finder will take precedence of the compiled module for loading, but you (griffe) can still just use the static .pyi file for documentation.

I will try and update the bug repo to show this pattern, which is what I want to use anyways!

@pawamoy
Copy link
Member

pawamoy commented May 12, 2023

Oh yes, definitely, that's a good idea! Griffe does support stubs (.pyi files) so this would work, and even give better result than relying on the inspection of the compiled modules (no need to import, so faster, and more accurate API signatures) 🙂

@JeremyBYU
Copy link
Author

I have pushed an update to the bug repo with a python stub file. Griffe is still currently not working with that solution as well. There are issues both in the local install and with the editable install (but failing for different reasons).

Editable:

Two problems:

  1. Griffe does not use the python stub file. It uses the docstrings from compiled module. I think this is a prioritization issue maybe?
  2. Griffe can not seem to find the pure python package now! Really weird not sure what the issue is:
❯ mkdocs serve
INFO     -  Building documentation...
INFO     -  Cleaning site directory
ERROR    -  mkdocstrings: griffedemo.purepython could not be found
ERROR    -  Error reading page 'purepython.md':
ERROR    -  Could not collect 'griffedemo.purepython'

Local Install:

One Problem:

  1. Griffe does not use the python stub file. It uses the docstrings from compiled module.

Thanks again for your help!

@pawamoy
Copy link
Member

pawamoy commented May 17, 2023

And thanks again for the update on the repro!

I removed my venv, recreated it, installed in editable mode, and the docs show up just fine: I see both the purepython solve_quadratic function, as well as the CompiledC++ add and substract methods, with sources coming from the stub file.

Now if I install in non-editable mode, it indeed seems to use the data from the compiled module, which is now found. To fix this, you can tell Griffe to avoid inspecting the compiled module, since you provide stubs with accurate data anyway:

plugins:
- mkdocstrings:
    handlers:
      python:
        options:
          docstring_style: google
          docstring_options:
            ignore_init_summary: yes
          merge_init_into_class: yes
          show_submodules: no
          allow_inspection: no

See https://mkdocstrings.github.io/python/usage/configuration/general/#allow_inspection.


About the original issue: I don't see a clean way to support how scikit-build-core puts files on the disk. Even using the Python's standard mechanisms to find packages won't allow us to discover all modules without actually importing some of them, which is a no-go for Griffe. I can still add support for scikit-build-core editable module names, but that will not help finding the compiled modules (only the sources .py files).

In any case, I believe that stubs are the way to go for documenting compiled modules.

@pawamoy
Copy link
Member

pawamoy commented May 17, 2023

Kay, released a new version, closing again (:joy:) , feel free to comment further :slightly_smiling_face:

@pawamoy pawamoy closed this as completed May 17, 2023
@JeremyBYU
Copy link
Author

Thank you again for your help. I followed suit and removed my old virtual environment and started fresh from my github repo master branch.

Your solution for allow_inpsection: no works perfectly for me when I do a local full install (pip install .). Everything is generated perfectly. However, I am still having issues in edit mode.

pip install -e .

❯ mkdocs serve --verbose
DEBUG    -  Loading configuration file: C:\Users\Jerem\Documents\Springfield\Research\griffe-scikit-build-core-bug\mkdocs.yml
DEBUG    -  Loaded theme configuration for 'mkdocs' from
            'C:\Users\Jerem\scoop\apps\mambaforge\current\envs\bug\lib\site-packages\mkdocs\themes\mkdocs\mkdocs_theme.yml': {'static_templates': ['404.html'],
            'locale': 'en', 'include_search_page': False, 'search_index_only': False, 'highlightjs': True, 'hljs_languages': [], 'hljs_style': 'github',
            'navigation_depth': 2, 'nav_style': 'primary', 'analytics': {'gtag': None}, 'shortcuts': {'help': 191, 'next': 78, 'previous': 80, 'search': 83}}
DEBUG    -  Config value 'config_file_path' = 'C:\\Users\\Jerem\\Documents\\Springfield\\Research\\griffe-scikit-build-core-bug\\mkdocs.yml'
DEBUG    -  Config value 'site_name' = 'My Docs'
DEBUG    -  Config value 'nav' = [{'Home': 'index.md'}, {'CompiledC++': 'compiled.md'}, {'PurePython': 'purepython.md'}]
DEBUG    -  Config value 'pages' = None
DEBUG    -  Config value 'site_url' = None
DEBUG    -  Config value 'site_description' = None
DEBUG    -  Config value 'site_author' = None
DEBUG    -  Config value 'theme' = Theme(name='mkdocs',
            dirs=['C:\\Users\\Jerem\\scoop\\apps\\mambaforge\\current\\envs\\bug\\lib\\site-packages\\mkdocs\\themes\\mkdocs',
            'C:\\Users\\Jerem\\scoop\\apps\\mambaforge\\current\\envs\\bug\\lib\\site-packages\\mkdocs\\templates'], static_templates=['sitemap.xml', '404.html'],
            name='mkdocs', locale=Locale(language='en', territory=''), include_search_page=False, search_index_only=False, highlightjs=True, hljs_languages=[],
            hljs_style='github', navigation_depth=2, nav_style='primary', analytics={'gtag': None}, shortcuts={'help': 191, 'next': 78, 'previous': 80, 'search': 83})
DEBUG    -  Config value 'docs_dir' = 'C:\\Users\\Jerem\\Documents\\Springfield\\Research\\griffe-scikit-build-core-bug\\docs'
DEBUG    -  Config value 'site_dir' = 'C:\\Users\\Jerem\\AppData\\Local\\Temp\\mkdocs_wih03hww'
DEBUG    -  Config value 'copyright' = None
DEBUG    -  Config value 'google_analytics' = None
DEBUG    -  Config value 'dev_addr' = _IpAddressValue(host='127.0.0.1', port=8000)
DEBUG    -  Config value 'use_directory_urls' = True
DEBUG    -  Config value 'repo_url' = None
DEBUG    -  Config value 'repo_name' = None
DEBUG    -  Config value 'edit_uri_template' = None
DEBUG    -  Config value 'edit_uri' = None
DEBUG    -  Config value 'extra_css' = []
DEBUG    -  Config value 'extra_javascript' = []
DEBUG    -  Config value 'extra_templates' = []
DEBUG    -  Config value 'markdown_extensions' = ['toc', 'tables', 'fenced_code']
DEBUG    -  Config value 'mdx_configs' = {}
DEBUG    -  Config value 'strict' = False
DEBUG    -  Config value 'remote_branch' = 'gh-pages'
DEBUG    -  Config value 'remote_name' = 'origin'
DEBUG    -  Config value 'extra' = {}
DEBUG    -  Config value 'plugins' = {'mkdocstrings': <mkdocstrings.plugin.MkdocstringsPlugin object at 0x0000024A4E53E140>}
DEBUG    -  Config value 'hooks' = {}
DEBUG    -  Config value 'watch' = []
INFO     -  Building documentation...
DEBUG    -  Running 1 `config` events
DEBUG    -  mkdocstrings: Adding extension to the list
DEBUG    -  mkdocstrings: Added a subdued autorefs instance <mkdocs_autorefs.plugin.AutorefsPlugin object at 0x0000024A4E5FACE0>
DEBUG    -  mkdocs_autorefs.plugin: Adding AutorefsExtension to the list
INFO     -  Cleaning site directory
DEBUG    -  Reading markdown pages.
DEBUG    -  Reading: index.md
DEBUG    -  Running 1 `page_markdown` events
DEBUG    -  Running 1 `page_content` events
DEBUG    -  Reading: compiled.md
DEBUG    -  Running 1 `page_markdown` events
DEBUG    -  mkdocstrings: Matched '::: griffedemo._core'
DEBUG    -  mkdocstrings: Using handler 'python'
DEBUG    -  mkdocstrings: Collecting data
DEBUG    -  griffe: Found griffedemo: loading
DEBUG    -  griffe: Loading path [WindowsPath('C:/Users/Jerem/scoop/apps/mambaforge/current/envs/bug/lib/site-packages/griffedemo')]
DEBUG    -  griffe: Loading path C:\Users\Jerem\scoop\apps\mambaforge\current\envs\bug\lib\site-packages\griffedemo\_core.cp310-win_amd64.pyd
DEBUG    -  griffe: Cannot load compiled module without inspection
DEBUG    -  griffe: Iteration 1 finished, 0 aliases resolved, still 0 to go
ERROR    -  mkdocstrings: griffedemo._core could not be found
ERROR    -  Error reading page 'compiled.md':
ERROR    -  Could not collect 'griffedemo._core'

To me, the issue still seems to be that its not looking at the local project folder to find the pure python code and the pyi stub file. Its still focused on the site_pacakges. If I remove compiled.md and see if griffe can make documentation just for the pure python I get this error:

DEBUG    -  griffe: Found griffedemo: loading
DEBUG    -  griffe: Loading path [WindowsPath('C:/Users/Jerem/scoop/apps/mambaforge/current/envs/bug/lib/site-packages/griffedemo')]
DEBUG    -  griffe: Loading path C:\Users\Jerem\scoop\apps\mambaforge\current\envs\bug\lib\site-packages\griffedemo\_core.cp310-win_amd64.pyd
DEBUG    -  griffe: Cannot load compiled module without inspection
DEBUG    -  griffe: Iteration 1 finished, 0 aliases resolved, still 0 to go
ERROR    -  mkdocstrings: griffedemo.purepython could not be found
ERROR    -  Error reading page 'purepython.md':
ERROR    -  Could not collect 'griffedemo.purepython'

Same issue as before when looking at the wrong path. Note that I am running windows 11, 64 bit, conda environments, python 3.10.

You have already put so much work into this issue that I recommend that you close this and forget about it. The stop gap for me is to simply install full locally before updating my documentation. When I am done, I just reinstall in editable mode. It's not that serious of an issue and this project is super helpful the way it currently works.

@pawamoy
Copy link
Member

pawamoy commented May 17, 2023

It seems like a difference between Windows and Linux. Here are the equivalent logs on Linux:

DEBUG    -  mkdocstrings: Matched '::: griffedemo._core'
DEBUG    -  mkdocstrings: Using handler 'python'
DEBUG    -  mkdocstrings: Collecting data
DEBUG    -  griffe: Found griffedemo: loading
DEBUG    -  griffe: Loading path /media/data/dev/griffe-scikit-build-core-bug/src/griffedemo/__init__.py
DEBUG    -  griffe: Loading path /media/data/dev/griffe-scikit-build-core-bug/src/griffedemo/_core.pyi
DEBUG    -  griffe: Loading path /media/data/dev/griffe-scikit-build-core-bug/src/griffedemo/purepython/__init__.py
DEBUG    -  griffe: Alias griffedemo.add was resolved to griffedemo._core.add
DEBUG    -  griffe: Alias griffedemo.subtract was resolved to griffedemo._core.subtract
DEBUG    -  griffe: Iteration 1 finished, 2 aliases resolved, still 2 to go
DEBUG    -  griffe: Iteration 2 finished, 0 aliases resolved, still 2 to go

Notice how your logs say Loading path [WindowsPath('C:/Users/Jerem/scoop/apps/mambaforge/current/envs/bug/lib/site-packages/griffedemo')], as if Griffe found the package to be a namespace package (list of paths), weird 🤔
I don't have access to a Windows machine so won't be able to debug this. Feel free to investigate further if you're willing to! I'll gladly fix any issue you find 🙂

@JeremyBYU
Copy link
Author

Haha , will do at some point! Thanks again for all your help in the open source community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants