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

MAINT: Guard type checking imports in TYPE_CHECKING block #739

Merged

Conversation

slangeveld
Copy link
Contributor

Resolves #725

Imports that are only used to annotate types in static type-checking, are moved into a type-checking block:

if TYPE_CHECKING:
    ...

The block ensures that these imports will only be imported when we are doing static type checking with mypy.

NB! As stated in #725 imports that are used by Pydantic at runtime can not be moved to the TYPE_CHECKING block

Copy link
Collaborator

@mferrera mferrera left a comment

Choose a reason for hiding this comment

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

LGTM 👍 remember to squash or rebase the fix-up commit before merging

Comment on lines -9 to -10
import pandas as pd
import xtgeo
Copy link
Collaborator

@mferrera mferrera Jul 4, 2024

Choose a reason for hiding this comment

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

This is a nice save, in theory. Both pandas and xtgeo can be slow to import (xtgeo also imports pandas), so whenever we can constrain them to being imported under a TYPE_CHECKING guard it's a good thing. You see this mostly during "start-up", just before execution is beginning, but with Python this can be quite noticeable (10s of seconds).

A big speed impediment to import time is Python compiling the interpreted code into byte code. Usually this means that the first import is slow, and the rest of the imports are "cached" in the sense that things have already been located and compiled, but whenever we can reduce the dependency graph we can make that compilation faster.

@mferrera
Copy link
Collaborator

mferrera commented Jul 5, 2024

I think you should also be able to guard from pydantic_core import CoreSchema.

core_schema: CoreSchema,

In these cases it is used only as an annotation to a method parameter, which Pydantic does not inspect. This will be a good dependency to hide for type checking only because pydantic-core is generally not meant to be used directly (it is the Rust-with-Python-bindings backend to Pydantic).

@slangeveld slangeveld force-pushed the use-typecheckingblocks-for-import branch from a7b5801 to 7bbfb24 Compare July 5, 2024 07:32
@slangeveld slangeveld merged commit 827f293 into equinor:main Jul 5, 2024
13 checks passed
@slangeveld slangeveld deleted the use-typecheckingblocks-for-import branch July 5, 2024 07:41
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.

Guard imports used only for typing checking under TYPE_CHECKING
2 participants