Repository navigation
Conversation
|
|
||
| @L.setter | ||
| def L(self, value): | ||
| def L(self, value: ScalarLike | Sequence[ScalarLike]) -> None: |
There was a problem hiding this comment.
In cases like this where the valid sequences have a fixed length, I think it might be better to use tuple[ScalarLike, ScalarLike, Scalarlike]. A little more verbose, but gives you meaningful feedback if you pass an incorrectly typed input
| def make_absolute(self, fractional_coordinates, out=None): | ||
| def make_absolute( | ||
| self, fractional_coordinates: ArrayLike, out: ArrayLike | None = None | ||
| ) -> npt.NDArray[np.floating]: |
There was a problem hiding this comment.
We defined FloatArray, could we use that as the return?
| return np.squeeze(out) if flatten else out | ||
|
|
||
| def get_images(self, vecs): | ||
| def get_images(self, vecs: ArrayLike) -> npt.NDArray[np.integer]: |
There was a problem hiding this comment.
And intarray -- I think we can swap these all out with ruplacer
| def __init__(self, box, basis_positions=None): | ||
| def __init__( | ||
| self, box: freud.box.Box | ArrayLike, basis_positions: ArrayLike | None = None | ||
| ) -> None: | ||
| if basis_positions is None: | ||
| basis_positions = [[0, 0, 0]] |
There was a problem hiding this comment.
Because we automatically convert basis_positions if it is None, could we instead set the default to basis_positions=((0,0,0),)?
There was a problem hiding this comment.
We could but this would technically be an API breaking change.....
There was a problem hiding this comment.
Looks like (nested) tuples are ArrayLike, but it’s no problem either way
| def to_image(self, cmap="afmhot", vmin=None, vmax=None): | ||
| def to_image( | ||
| self, cmap: str = "afmhot", vmin: float | None = None, vmax: float | None = None | ||
| ) -> np.ndarray: |
There was a problem hiding this comment.
| ) -> np.ndarray: | |
| ) -> npt.NDArray[np.uint8]: |
Full shape annotation is not supported but it's nice to mark this as uint8 imo
| NeighborQueryDict = dict[str, object] | ||
|
|
||
|
|
||
| class _CPPNPoint(Protocol): | ||
| def getQueryPointIdx(self) -> int: ... | ||
| def getPointIdx(self) -> int: ... | ||
| def getDistance(self) -> float: ... | ||
|
|
||
|
|
||
| class _CPPNeighborList(Protocol): | ||
| def copy(self, other: object) -> None: ... | ||
| def getNeighbors(self) -> _CPPIntArray: ... | ||
| def getWeights(self) -> _CPPFloatArray: ... | ||
| def getDistances(self) -> _CPPFloatArray: ... | ||
| def getVectors(self) -> _CPPFloatArray: ... | ||
| def getSegments(self) -> _CPPIntArray: ... | ||
| def getCounts(self) -> _CPPIntArray: ... | ||
| def getNumBonds(self) -> int: ... | ||
| def getNumQueryPoints(self) -> int: ... | ||
| def getNumPoints(self) -> int: ... | ||
| def find_first_index(self, i: int) -> int: ... | ||
| def filter(self, filt: ArrayLike) -> None: ... | ||
| def filter_r(self, r_max: ScalarLike, r_min: ScalarLike) -> None: ... | ||
| def sort(self, by_distance: bool) -> None: ... | ||
|
|
||
|
|
||
| class _CPPNeighborIterator(Protocol): | ||
| def next(self) -> _CPPNPoint: ... | ||
| def toNeighborList(self, sort_by_distance: bool) -> _CPPNeighborList: ... | ||
|
|
||
|
|
||
| class _CPPNeighborQuery(Protocol): | ||
| def query(self, points: ArrayLike, query_args: object) -> _CPPNeighborIterator: ... | ||
| def getBox(self) -> freud.box._CPPBox: ... | ||
|
|
||
|
|
||
| class _CPPLinkCell(_CPPNeighborQuery, Protocol): | ||
| def getCellWidth(self) -> float: ... | ||
|
|
||
|
|
||
| class _CPPFloatArray(Protocol): | ||
| def toNumpyArray(self) -> npt.NDArray[np.floating]: ... | ||
|
|
||
|
|
||
| class _CPPIntArray(Protocol): | ||
| def toNumpyArray(self) -> npt.NDArray[np.integer]: ... | ||
|
|
||
|
|
||
| class _CPPSpatialHistogram(Protocol): | ||
| mode: int | ||
|
|
||
| def getBox(self) -> freud.box._CPPBox: ... | ||
| def getBinCounts(self) -> _CPPFloatArray: ... | ||
| def getBinCenters(self) -> Sequence[npt.NDArray[np.floating]]: ... | ||
| def getBinEdges(self) -> Sequence[npt.NDArray[np.floating]]: ... | ||
| def getBounds(self) -> list[tuple[float, float]] | tuple[float, float]: ... | ||
| def getAxisSizes(self) -> list[int] | int: ... | ||
| def reset(self) -> None: ... | ||
| def accumulate(self, *args: object) -> None: ... | ||
| def getCorrelation(self) -> _CPPFloatArray: ... | ||
| def getNBins(self) -> int: ... | ||
| def accumulateRDF(self, *args: object) -> None: ... | ||
| def getRDF(self) -> _CPPFloatArray: ... | ||
| def getNr(self) -> _CPPFloatArray: ... | ||
| def getBondOrder(self) -> _CPPFloatArray: ... | ||
| def getMode(self) -> _CPPModeEnum: ... | ||
| def getPCF(self) -> _CPPFloatArray: ... | ||
|
|
||
|
|
||
| class _CPPModeEnum(Protocol): | ||
| name: str | ||
|
|
||
|
|
||
| class _CPPVoronoi(Protocol): | ||
| def compute(self, query: _CPPNeighborQuery) -> None: ... | ||
| def getPolytopes(self) -> Sequence[Sequence[Sequence[float]]]: ... | ||
| def getVolumes(self) -> _CPPFloatArray: ... | ||
| def getNeighborList(self) -> _CPPNeighborList: ... | ||
|
|
||
|
|
||
| class _CPPFilter(Protocol): | ||
| def compute( | ||
| self, | ||
| query: _CPPNeighborQuery, | ||
| query_points: FloatArray, | ||
| nlist: _CPPNeighborList, | ||
| qargs: object, | ||
| ) -> None: ... | ||
| def getFilteredNlist(self) -> _CPPNeighborList: ... | ||
| def getUnfilteredNlist(self) -> _CPPNeighborList: ... | ||
|
|
||
|
|
||
| class _MDATimestep(Protocol): | ||
| triclinic_dimensions: ArrayLike | ||
| positions: ArrayLike | ||
|
|
||
|
|
||
| class _ConfigurationLike(Protocol): | ||
| box: ArrayLike | ||
| dimensions: int | ||
|
|
||
|
|
||
| class _ParticlesWithPosition(Protocol): | ||
| position: ArrayLike | ||
|
|
||
|
|
||
| class _SnapshotLike(Protocol): | ||
| configuration: _ConfigurationLike | ||
| particles: _ParticlesWithPosition | ||
|
|
||
|
|
||
| class _GarnettFrame(Protocol): | ||
| box: object | ||
| positions: ArrayLike | ||
|
|
||
|
|
||
| class _GarnettFrameNew(_GarnettFrame, Protocol): | ||
| position: ArrayLike | ||
|
|
||
|
|
||
| class _Hoomd2Box(Protocol): | ||
| dimensions: int | ||
| Lx: float | ||
| Ly: float | ||
| xy: float | ||
|
|
||
|
|
||
| class _Hoomd2Particles(Protocol): | ||
| position: ArrayLike | ||
|
|
||
|
|
||
| class _Hoomd2Snapshot(Protocol): | ||
| box: _Hoomd2Box | ||
| particles: _Hoomd2Particles | ||
|
|
||
|
|
||
| class _OvitoCell(Protocol): | ||
| matrix: FloatArray | ||
| is2D: bool # noqa: N815 - external API naming | ||
|
|
||
|
|
||
| class _OvitoParticles(Protocol): | ||
| positions: ArrayLike | ||
|
|
||
|
|
||
| class _OvitoCollection(Protocol): | ||
| cell: _OvitoCell | ||
| particles: _OvitoParticles | ||
|
|
||
|
|
||
| class _SystemWithBoxPoints(Protocol): | ||
| box: object | ||
| points: ArrayLike |
There was a problem hiding this comment.
We could consider moving this to a freud/locality_typing.py or similar?
| import inspect | ||
| from collections.abc import Iterator, Mapping, Sequence | ||
| from importlib.util import find_spec | ||
| from typing import TYPE_CHECKING, Literal, Protocol, cast |
There was a problem hiding this comment.
Even in Python we end up with reinterpret cast, it truly is inevitable
|
I'm generally happy with these changes, just a few consistency questions and a nitpick here or there. This will be quite useful to have, it's remarkable to see how far python typing has come. As a quick heads-up: while the typing is correct at a hight level there are many cases where we don't formally satisfy all the requirements. I don't think this really matters but it does cause a lot of typing errors in LSPs: 
|
|
Looks like we can fix some module-not-found errors (for the freud._* pattern) using nanobind stubgen, if we're ok adding a bunch of .pyi files to the repo |
…o feat/add_type_hints
We should consider this for a future PR. Most of user facing functions are wrapped in python code so at first glance, I don't think users would benefit much from them. Did you have some concrete situation in mind where having them would be helpful? |
janbridley
left a comment
janbridley
left a comment
There was a problem hiding this comment.
Looks great! This will be very nice to have
|
@janbridley Can you take another look at the most current changes please? |
This PR adds missing type hinting for Python function returns.
Description
Many Python functions have missing type hints.
Motivation and Context
Resolves: #1311
How Has This Been Tested?
Locally run mypy succesfully.
Checklist: