From 6fa05152bab595439800a9161e5be89a799978fd Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Wed, 28 Aug 2024 10:39:57 +0200 Subject: [PATCH] Review suggestions --- README.md | 8 +++--- parcels/_typing.py | 53 +++++++++++++++++-------------------- parcels/field.py | 14 +++++----- parcels/fieldfilebuffer.py | 10 ++++++- parcels/fieldset.py | 18 ++++++------- parcels/tools/converters.py | 4 +-- 6 files changed, 55 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index 1e155dcf7..6ddac8f81 100644 --- a/README.md +++ b/README.md @@ -1,13 +1,13 @@ ## Parcels -[![Binder](https://mybinder.org/badge_logo.svg)](https://mybinder.org/v2/gh/OceanParcels/parcels/master?labpath=docs%2Fexamples%2Fparcels_tutorial.ipynb) -[![unit-tests](https://github.com/OceanParcels/parcels/actions/workflows/ci.yml/badge.svg)](https://github.com/OceanParcels/parcels/actions/workflows/ci.yml) -[![Code style: Ruff](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ruff/main/assets/badge/format.json)](https://github.com/astral-sh/ruff) -[![codecov](https://codecov.io/gh/OceanParcels/parcels/branch/master/graph/badge.svg)](https://codecov.io/gh/OceanParcels/parcels) [![Anaconda-release](https://anaconda.org/conda-forge/parcels/badges/version.svg)](https://anaconda.org/conda-forge/parcels/) [![Anaconda-date](https://anaconda.org/conda-forge/parcels/badges/latest_release_date.svg)](https://anaconda.org/conda-forge/parcels/) [![Zenodo](https://zenodo.org/badge/DOI/10.5281/zenodo.823561.svg)](https://doi.org/10.5281/zenodo.823561) +[![Code style: Ruff](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ruff/main/assets/badge/format.json)](https://github.com/astral-sh/ruff) +[![unit-tests](https://github.com/OceanParcels/parcels/actions/workflows/ci.yml/badge.svg)](https://github.com/OceanParcels/parcels/actions/workflows/ci.yml) +[![codecov](https://codecov.io/gh/OceanParcels/parcels/branch/master/graph/badge.svg)](https://codecov.io/gh/OceanParcels/parcels) [![CII Best Practices](https://bestpractices.coreinfrastructure.org/projects/5353/badge)](https://bestpractices.coreinfrastructure.org/projects/5353) +[![Binder](https://mybinder.org/badge_logo.svg)](https://mybinder.org/v2/gh/OceanParcels/parcels/master?labpath=docs%2Fexamples%2Fparcels_tutorial.ipynb) **Parcels** (**P**robably **A** **R**eally **C**omputationally **E**fficient **L**agrangian **S**imulator) is a set of Python classes and methods to create customisable particle tracking simulations using output from Ocean Circulation models. Parcels can be used to track passive and active particulates such as water, plankton, [plastic](http://www.topios.org/) and [fish](https://github.com/Jacketless/IKAMOANA). diff --git a/parcels/_typing.py b/parcels/_typing.py index f67192bcb..2e7ace119 100644 --- a/parcels/_typing.py +++ b/parcels/_typing.py @@ -9,42 +9,37 @@ import ast import datetime import os -from typing import Any, Callable, Literal, get_args +from typing import Callable, Literal class ParcelsAST(ast.AST): ccode: str -# InterpMethod = InterpMethodOption | dict[str, InterpMethodOption] # (can also be a dict, search for `if type(interp_method) is dict`) -# InterpMethodOption = Literal[ -# "nearest", -# "freeslip", -# "partialslip", -# "bgrid_velocity", -# "bgrid_w_velocity", -# "cgrid_velocity", -# "linear_invdist_land_tracer", -# "nearest", -# "cgrid_tracer", -# ] # mostly corresponds with `interp_method` # TODO: This should be narrowed. Unlikely applies to every context +InterpMethodOption = Literal[ + "linear", + "nearest", + "freeslip", + "partialslip", + "bgrid_velocity", + "bgrid_w_velocity", + "cgrid_velocity", + "linear_invdist_land_tracer", + "nearest", + "bgrid_tracer", + "cgrid_tracer", +] # corresponds with `tracer_interp_method` +InterpMethod = ( + InterpMethodOption | dict[str, InterpMethodOption] +) # corresponds with `interp_method` (which can also be dict mapping field names to method) PathLike = str | os.PathLike -Mesh = Literal["spherical", "flat"] # mostly corresponds with `mesh` -VectorType = Literal["3D", "2D"] | None # mostly corresponds with `vector_type` -ChunkMode = Literal["auto", "specific", "failsafe"] # mostly corresponds with `chunk_mode` -GridIndexingType = Literal["pop", "mom5", "mitgcm", "nemo"] # mostly corresponds with `grid_indexing_type` -UpdateStatus = Literal["not_updated", "first_updated", "updated"] # mostly corresponds with `update_status` -TimePeriodic = float | datetime.timedelta | Literal[False] # mostly corresponds with `update_status` -NetcdfEngine = Literal[ - "netcdf4", "xarray" -] # TODO: It seems that "scipy" is also an option (according to a docstring) but can't find mention in code. Investigate. +Mesh = Literal["spherical", "flat"] # corresponds with `mesh` +VectorType = Literal["3D", "2D"] | None # corresponds with `vector_type` +ChunkMode = Literal["auto", "specific", "failsafe"] # corresponds with `chunk_mode` +GridIndexingType = Literal["pop", "mom5", "mitgcm", "nemo"] # corresponds with `grid_indexing_type` +UpdateStatus = Literal["not_updated", "first_updated", "updated"] # corresponds with `update_status` +TimePeriodic = float | datetime.timedelta | Literal[False] # corresponds with `update_status` +NetcdfEngine = Literal["netcdf4", "xarray", "scipy"] KernelFunction = Callable[..., None] - - -def ensure_is_literal_value(value: Any, literal: Any) -> None: - """Ensures that a value is a valid option for the provided Literal type annotation.""" - valid_options = get_args(literal) - if value not in valid_options: - raise ValueError(f"{value!r} is not a valid option. Valid options are {valid_options}") diff --git a/parcels/field.py b/parcels/field.py index 7495ccdd1..aa36fad46 100644 --- a/parcels/field.py +++ b/parcels/field.py @@ -10,7 +10,7 @@ import xarray as xr import parcels.tools.interpolation_utils as i_u -from parcels._typing import GridIndexingType, Mesh, TimePeriodic, VectorType +from parcels._typing import GridIndexingType, InterpMethod, Mesh, TimePeriodic, VectorType from parcels.tools.converters import ( Geographic, GeographicPolar, @@ -156,7 +156,7 @@ def __init__( vmax=None, cast_data_dtype="float32", time_origin=None, - interp_method="linear", + interp_method: InterpMethod = "linear", allow_time_extrapolation: bool | None = None, time_periodic: TimePeriodic = False, gridindexingtype: GridIndexingType = "nemo", @@ -199,7 +199,7 @@ def __init__( else: raise ValueError("Unsupported mesh type. Choose either: 'spherical' or 'flat'") self.timestamps = timestamps - if type(interp_method) is dict: + if isinstance(interp_method, dict): if self.name in interp_method: self.interp_method = interp_method[self.name] else: @@ -215,7 +215,7 @@ def __init__( "General s-levels are not supported in B-grid. RectilinearSGrid and CurvilinearSGrid can still be used to deal with shaved cells, but the levels must be horizontal." ) - self.fieldset: FieldSet | None = None + self.fieldset: "FieldSet" | None = None if allow_time_extrapolation is None: self.allow_time_extrapolation = True if len(self.grid.time) == 1 else False else: @@ -292,7 +292,7 @@ def __init__( # since some datasets do not provide the deeper level of data (which is ignored by the interpolation). self.data_full_zdim = kwargs.pop("data_full_zdim", None) self.data_chunks = [] # type: ignore # the data buffer of the FileBuffer raw loaded data - shall be a list of C-contiguous arrays - self.c_data_chunks: list[PointerType | None] = [] # C-pointers to the data_chunks array + self.c_data_chunks: list["PointerType" | None] = [] # C-pointers to the data_chunks array self.nchunks: tuple[int, ...] = () self.chunk_set: bool = False self.filebuffers = [None] * 2 @@ -489,7 +489,7 @@ def from_netcdf( "if you would need such feature implemented." ) - interp_method = kwargs.pop("interp_method", "linear") + interp_method: InterpMethod = kwargs.pop("interp_method", "linear") if type(interp_method) is dict: if variable[0] in interp_method: interp_method = interp_method[variable[0]] @@ -871,7 +871,7 @@ def search_indices_vertical_z(self, z): return (zi, zeta) def search_indices_vertical_s( - self, x: float, y: float, z: float, xi: int, yi: int, xsi: float, eta: float, ti: int, time + self, x: float, y: float, z: float, xi: int, yi: int, xsi: float, eta: float, ti: int, time: float ): grid = self.grid if self.interp_method in ["bgrid_velocity", "bgrid_w_velocity", "bgrid_tracer"]: diff --git a/parcels/fieldfilebuffer.py b/parcels/fieldfilebuffer.py index dd702270a..c3b45fc51 100644 --- a/parcels/fieldfilebuffer.py +++ b/parcels/fieldfilebuffer.py @@ -9,6 +9,7 @@ from dask import utils as da_utils from netCDF4 import Dataset as ncDataset +from parcels._typing import InterpMethodOption from parcels.tools.converters import convert_xarray_time_units from parcels.tools.loggers import logger from parcels.tools.statuscodes import DaskChunkingError @@ -16,7 +17,14 @@ class _FileBuffer: def __init__( - self, filename, dimensions, indices, timestamp=None, interp_method="linear", data_full_zdim=None, **kwargs + self, + filename, + dimensions, + indices, + timestamp=None, + interp_method: InterpMethodOption = "linear", + data_full_zdim=None, + **kwargs, ): self.filename = filename self.dimensions = dimensions # Dict with dimension keys for file data diff --git a/parcels/fieldset.py b/parcels/fieldset.py index b47713b54..16b690f5c 100644 --- a/parcels/fieldset.py +++ b/parcels/fieldset.py @@ -8,7 +8,7 @@ import numpy as np from parcels._compat import MPI -from parcels._typing import Mesh, TimePeriodic +from parcels._typing import GridIndexingType, InterpMethodOption, Mesh, TimePeriodic from parcels.field import DeferredArray, Field, NestedField, VectorField from parcels.grid import Grid from parcels.gridset import GridSet @@ -415,7 +415,7 @@ def from_netcdf( ``{parcels_varname: {netcdf_dimname : (parcels_dimname, chunksize_as_int)}, ...}``, where ``parcels_dimname`` is one of ('time', 'depth', 'lat', 'lon') netcdf_engine : engine to use for netcdf reading in xarray. Default is 'netcdf', - but in cases where this doesn't work, setting netcdf_engine='scipy' could help + but in cases where this doesn't work, setting netcdf_engine='scipy' could help. Accepted options are the same as the ``engine`` parameter in ``xarray.open_dataset()``. **kwargs : Keyword arguments passed to the :class:`parcels.Field` constructor. @@ -524,7 +524,7 @@ def from_nemo( mesh: Mesh = "spherical", allow_time_extrapolation: bool | None = None, time_periodic: TimePeriodic = False, - tracer_interp_method="cgrid_tracer", + tracer_interp_method: InterpMethodOption = "cgrid_tracer", chunksize=None, **kwargs, ): @@ -635,7 +635,7 @@ def from_mitgcm( mesh: Mesh = "spherical", allow_time_extrapolation: bool | None = None, time_periodic: TimePeriodic = False, - tracer_interp_method="cgrid_tracer", + tracer_interp_method: InterpMethodOption = "cgrid_tracer", chunksize=None, **kwargs, ): @@ -685,8 +685,8 @@ def from_c_grid_dataset( mesh: Mesh = "spherical", allow_time_extrapolation: bool | None = None, time_periodic: TimePeriodic = False, - tracer_interp_method="cgrid_tracer", - gridindexingtype="nemo", + tracer_interp_method: InterpMethodOption = "cgrid_tracer", + gridindexingtype: GridIndexingType = "nemo", chunksize=None, **kwargs, ): @@ -807,7 +807,7 @@ def from_pop( mesh: Mesh = "spherical", allow_time_extrapolation: bool | None = None, time_periodic: TimePeriodic = False, - tracer_interp_method="bgrid_tracer", + tracer_interp_method: InterpMethodOption = "bgrid_tracer", chunksize=None, depth_units="m", **kwargs, @@ -928,7 +928,7 @@ def from_mom5( mesh: Mesh = "spherical", allow_time_extrapolation: bool | None = None, time_periodic: TimePeriodic = False, - tracer_interp_method="bgrid_tracer", + tracer_interp_method: InterpMethodOption = "bgrid_tracer", chunksize=None, **kwargs, ): @@ -1052,7 +1052,7 @@ def from_b_grid_dataset( mesh: Mesh = "spherical", allow_time_extrapolation: bool | None = None, time_periodic: TimePeriodic = False, - tracer_interp_method="bgrid_tracer", + tracer_interp_method: InterpMethodOption = "bgrid_tracer", chunksize=None, **kwargs, ): diff --git a/parcels/tools/converters.py b/parcels/tools/converters.py index f657f593b..de6705fac 100644 --- a/parcels/tools/converters.py +++ b/parcels/tools/converters.py @@ -5,8 +5,8 @@ import cftime import numpy as np +import numpy.typing as npt import xarray as xr -from numpy.typing import ArrayLike, NDArray __all__ = [ "UnitConverter", @@ -21,7 +21,7 @@ ] -def convert_to_flat_array(var: ArrayLike) -> NDArray: +def convert_to_flat_array(var: npt.ArrayLike) -> npt.NDArray: """Convert lists and single integers/floats to one-dimensional numpy arrays Parameters