Skip to content

feat: Custom base class detection #30

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sneakers-the-rat
Copy link

@sneakers-the-rat sneakers-the-rat commented Apr 16, 2025

Fix: #27

Here's an attempt at #27. If the approach is OK, i'll write the docs for it :)

New to mkdocs, so sorry if i did this wrong.

The gist is rather than keeping an infinite list of all the possible things that could be pydantic models, we just allow someone to specify a non-standard base for detection of e.g. sqlmodel, pydantic-settings, and any other wrappers. hopefully this saves us from having to import them (?)

Two ways:

Adding to the defaults

extensions:
  - griffe_pydantic:
      include_bases: ["mypackage.MyBase"]

Replacing the defaults (if we for some reason didn't want to document regular pydantic models and only wanted to document our special kind

extensions:
  - griffe_pydantic:
      bases: ["mypackage.MyBase"]

edit: need to fix that docs build check but i'm not exactly sure what the configuration error is from the message. sorry for the red-x-open, will get to this tmrw

@pawamoy
Copy link
Member

pawamoy commented Apr 21, 2025

Thanks for the PR @sneakers-the-rat 🚀

However there's a simpler solution: instead of iterating on cls.bases in static.py, _inherits_pydantic() function, we could iterate on cls.mro(), and tell users to preload SQLModel or any other package that define classes inheriting from Pydantic's base model class.

plugins:
- mkdocstrings:
    handlers:
      python:
        options:
          preload_modules:
          - sqlmodel

@pawamoy
Copy link
Member

pawamoy commented Apr 21, 2025

Oh wait, that's what we already do, actually.

def _inherits_pydantic(cls: Class) -> bool:
    for base in cls.bases:
        if isinstance(base, (ExprName, Expr)):
            base = base.canonical_path  # noqa: PLW2901
        if base in {"pydantic.BaseModel", "pydantic.main.BaseModel"}:
            return True
    return any(_inherits_pydantic(parent_class) for parent_class in cls.mro())

So we just need to update the documentation IMO.

Could you try preloading sqlmodel and see if that's solves your issue?

@sneakers-the-rat
Copy link
Author

Ah, ill give that a shot. I figured there was a difference between static and dynamic modes, where we would need to make the bases statically available in some contexts and dynamically in others, but now that I'm looking again it seems like we will always be importing anyway. Oh well still a good learning experience, hadn't tinkered around with a mkdocs plugin before, very tidy system compared to sphinx.

If that works I'll close this and open another with a docs update

@sneakers-the-rat
Copy link
Author

ok good news and bad news

  • i can get griffe-pydantic to render pydantic-settings classes by preloading the module
  • i can't get griffe-pydantic to render sqlmodel classes even with the preloaded modules with the main branch version. I tried a variety of settings for rendering inherited members, using the inherited docstrings, plugins, etc. and couldn't get it to go . I think this is because sqlmodel replaces the metaclass and does other funny stuff with model creation. I think we'll need to be able to specify explicit base classes like this PR (i was able to render sqlmodel classes with this PR)
  • griffe-pydantic seems to want to render all of the dunder attrs and methods even if they are explicitly excluded by filters (e.g. __pydantic_extra__, __pydantic_fields_set__, etc.).
  • This causes it to error when rendering because the template expects the field.annotation to not be None, but e.g. __slots__ has no annotation.

So it seems like we'll need this PR plus a few more to be able to properly handle both of them. Maybe for this PR i can add some notes to the docs like "for most cases, preloading the module should work to get it to be recognized, but for some others that manipulate model creation, you need to explicitly declare the base classes like..." Then i can follow on with another for excluding the dunder methods from the fields statement?

@pawamoy
Copy link
Member

pawamoy commented Apr 22, 2025

SQLModel does inherit from BaseModel though, so static analysis should detect it.

https://github.com/fastapi/sqlmodel/blob/main/sqlmodel/main.py#L770

I figured there was a difference between static and dynamic modes, where we would need to make the bases statically available in some contexts and dynamically in others, but now that I'm looking again it seems like we will always be importing anyway.

There is indeed a difference between static and dynamic analysis. Static analysis doesn't import anything and only infers inheritance from the sources, while dynamic analysis does import classes to inspect their bases.

Oh well still a good learning experience, hadn't tinkered around with a mkdocs plugin before, very tidy system compared to sphinx.

Thanks! Worth noting though is that we're solely using the Griffe extension system here, which knows nothing about MkDocs and its plugin system 🙂

flowchart TD
mkd["MkDocs"]
mkds["mkdocstrings (MkDocs plugin)"]
mkdsp["mkdocstrings-python (mkdocstrings handler)"]
g["Griffe (API data extractor for Python)"]
gp["griffe-pydantic (Griffe extension)"]

mkd --> mkds
mkds --> mkdsp
mkdsp --> g
g --> gp
Loading

Lets try to stay focused on one item at once. This PR is about supporting pydantic-settings, solving #27, right? So lets try to achieve that without concerning ourselves with SQLModel yet. You reported another issue with dunder fields: could you please open a new issue, if it makes sense?

@sneakers-the-rat
Copy link
Author

Good call. Ok I'll focus this on pydantic-settings, which I think just needs docs, open another issue, and then work on sqlmodel in another pr

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

Successfully merging this pull request may close these issues.

feature: support of pydantic-settings
2 participants