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

Reduce type resolution complexity #1060

Open
ezorita opened this issue Jan 16, 2025 · 5 comments
Open

Reduce type resolution complexity #1060

ezorita opened this issue Jan 16, 2025 · 5 comments

Comments

@ezorita
Copy link
Contributor

ezorita commented Jan 16, 2025

Problem

We are working on a medium-size project (our schema.prisma is < 400 lines) that generates a types.py file over 40.000 lines. This file is so big that Pylance refuses to process it, and hence we don't have type resolution while coding:

Code is too complex to analyze; reduce complexity by refactoring into subroutines or reducing conditional code paths

Suggested solution

I suggest splitting the types file into multiple files, e.g. one per Model.

Alternatives

Suggest Pylance settings that would allow processing such large file (I could not find any setting to fix it).

Additional context

image

@Pwuts
Copy link

Pwuts commented Jan 16, 2025

The maintainer of Pyright (the underlying type checker that powers Pylance) just posted a detailed explanation of why Pylance and Pyright can't handle these files: microsoft/pyright#9709 (comment)

Highlights

The code within this file's global (module) scope is exceeding pyright's internal threshold for cyclomatic complexity and statement count. Beyond this threshold, pyright will refuse to perform any sort of analysis on the code flow graph.

This limit wouldn't be an issue if the file contained only class definitions because pyright can evaluate a class definition without performing any code flow analysis. However, the prisma module is defining _AgentGraphCompoundPrimaryKey using the "alternate functional syntax" rather than using a standard class statement and then it's aliasing it to another variable AgentGraphWhereUniqueInput. I'm not sure why this approach was chosen given that most of the TypedDict classes in this file are defined using a class statement. The alternative syntax and the type alias definition are syntactically indistinguishable from a normal variable assignment, so pyright needs to perform code flow analysis to determine the variables' types. That's why you see this problem specifically for AgentGraphWhereUniqueInput but not for other TypedDict classes that are defined using a class statement.

Example of the mentioned "alternative functional syntax"

The "alternative functional syntax" definition of _AgentGraphCompoundPrimaryKey:

_AgentGraphCompoundPrimaryKeyInner = TypedDict(
    '_AgentGraphCompoundPrimaryKeyInner',
    {
        'id': '_str',
        'version': '_int',
    },
    total=True
)

_AgentGraphCompoundPrimaryKey = TypedDict(
    '_AgentGraphCompoundPrimaryKey',
    {
        'graphVersionId': '_AgentGraphCompoundPrimaryKeyInner',
    },
    total=True
)

AgentGraphWhereUniqueInput = _AgentGraphCompoundPrimaryKey

You may want to report the issue to the maintainers of prisma. Since this file is auto-generated, I'm hoping that it's straightforward for them to break it up into a more manageable submodules. Alternately, if they could generate class statements for all of their TypedDicts rather than using the alternate functional syntax, that would mitigate the issue.

Offered solutions

  • Define all TypedDict types with class statements rather than the functional syntax _SomeModelCompoundPrimaryKey = TypedDict(...)
  • Break up types.py into multiple files

@ezorita
Copy link
Contributor Author

ezorita commented Jan 17, 2025

Excellent point @Pwuts. I think the class statement definition is easier to handle. Updating the templates to render classes should be easier than breaking the code down into different files and handling the imports dynamically.

@ezorita
Copy link
Contributor Author

ezorita commented Jan 18, 2025

I did a local test, updated the types.py template to generate all valid class name(TypeDict, total=...). Found the following issues:

  • PyRight still complains the file is too complex.
  • There are cases in which the dict key is a reserved python word (like not), that can only be resolved with functional syntax.

@ezorita
Copy link
Contributor Author

ezorita commented Jan 18, 2025

The other possible solution is to split the types.py into multiple files. I bet this is possible but quite challenging. In a naive approach I created two templates:

  • one for recursive types and filter type definitions
  • the other to generate all types associated with a specific model

By modifying slightly the template generator I managed to generate one {model.name}_types.py file for each model.

Once each model lives in a different file, we need to resolve the dependencies among them (e.g. model A relies on types from model B if it has a foreign relation). Models that rely on each other would cause a circular import. To avoid this, we'd need to split files considering a second dimension that avoids circular imports (e.g. at least separate the *WithoutRelations* definitions in different files that are in turn imported).

@Pwuts
Copy link

Pwuts commented Jan 21, 2025

Models that rely on each other would cause a circular import. To avoid this, we'd need to split files considering a second dimension that avoids circular imports

Or do the imports at a different level, e.g. inside the class that uses it?

In any case, it seems like we're hitting the limits of generating Python modules with jinja templates.

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