-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add shapely stubs #12033
Add shapely stubs #12033
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glancing through this, I see no red flags. Two optional suggestions below.
stubs/shapely/shapely/__init__.pyi
Outdated
from .set_operations import * | ||
from .strtree import * | ||
|
||
__version__: str |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit (needs import):
__version__: str | |
__version__: Literal[str] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you meant Final
. Fixed.
stubs/shapely/shapely/_typing.pyi
Outdated
@type_check_only | ||
class SupportsRead(Protocol[_T_co]): | ||
def read(self) -> _T_co: ... | ||
|
||
@type_check_only | ||
class SupportsWrite(Protocol[_T_contra]): | ||
def write(self, s: _T_contra, /) -> object: ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering that these are copies from _typeshed
, maybe we should just re-export them? (Not that it matters terribly.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I deleted these definitions and used the typeshed one's instead. There is no need to redefine them.
Diff from mypy_primer, showing the effect of this PR on open source code: psycopg (https://github.com/psycopg/psycopg)
+ psycopg/psycopg/types/shapely.py:47: error: Unused "type: ignore" comment [unused-ignore]
+ psycopg/psycopg/types/shapely.py:52: error: Unused "type: ignore" comment [unused-ignore]
|
I basically copied the stubs from https://github.com/hamdanal/python-stubs/tree/5ecdeaede3a5d97b00b5d4298f13b7028c01e6eb/stubs/shapely-stubs
Note that these stubs are somewhat complete as I've been working on them for quiet some time. There is an extensive test suite which passes both mypy and pyright in addition to runtime type checking. I've also been testing those stubs in VS Code (through Pylance, no actual type checking) for a while and they seem to work as expected. Hopefully this will make the review a bit easier.
I haven't ported any of the tests to typeshed for now. I might add a subset of them later to avoid regressions.
Technical notes:
BaseGeometry
->Geometry
->object
. For input types, I useshapely.Geometry
as much as possible. For output types, I useshapely.geometry.BaseGeometry
. This is becauseBaseGeometry
is an implementation detail that should not be required to be imported by users -- they should be able to annotate their code usingGeometry
. For output types,Geometry
is not very useful as it doesn't have any attributes or methods.Related: shapely/shapely#1741 (comment)