Review feedback

Author: VeckoTheGecko

src/parcels/_core/particlefile.py

@@ -2,11 +2,10 @@
 
 from __future__ import annotations
 
-from datetime import datetime, timedelta
+from datetime import timedelta
 from pathlib import Path
 from typing import TYPE_CHECKING, Any, Literal
 
-import cftime
 import numpy as np
 import pandas as pd
 import pyarrow as pa
@@ -52,10 +51,8 @@ class ParticleFile:
 
     Parameters
     ----------
-    name : str
-        Basename of the output file. This can also be a Zarr store object.
-    particleset :
-        ParticleSet to output
+    path : PathLike
+        Path of the output Parquet file.
     outputdt :
         Interval which dictates the update frequency of file output
         while ParticleFile is given as an argument of ParticleSet.execute()
@@ -94,12 +91,8 @@ def __init__(self, path: PathLike, outputdt):
         if not path.parent.exists():
             raise ValueError(f"Folder location for {path=!r} does not exist. Create the folder location first.")
 
-        self._maxids = 0
-        self._pids_written = {}
         self.extra_metadata = {}
 
-        # TODO v4: Add check that if create_new_zarrfile is False, the store already exists
-
     def __repr__(self) -> str:
         return particlefile_repr(self)
 
@@ -155,9 +148,6 @@ def write(self, pset: ParticleSet, time, indices=None):
             pa.table({v.name: pa.array(particle_data[v.name][indices_to_write]) for v in vars_to_write}),
         )
 
-        # if len(indices_to_write) == 0: # TODO: Remove this?
-        #     return
-
     def close(self):
         if self._writer is not None:
             self._writer.close()
@@ -192,25 +182,6 @@ def _to_write_particles(particle_data, time):
     )[0]
 
 
-def _get_calendar_and_units(time_interval: TimeInterval) -> dict[str, str]:  # TODO: Remove?
-    calendar = None
-    units = "seconds"
-    if time_interval:
-        if isinstance(time_interval.left, (np.datetime64, datetime)):
-            calendar = "standard"
-        elif isinstance(time_interval.left, cftime.datetime):
-            calendar = time_interval.left.calendar
-
-    if calendar is not None:
-        units += f" since {time_interval.left}"
-
-    attrs = {"units": units}
-    if calendar is not None:
-        attrs["calendar"] = calendar
-
-    return attrs
-
-
 def read_particlefile(path: PathLike, decode_times: bool = True) -> pd.DataFrame:
     """Read a Parcels particlefile (Parquet format) into a pandas DataFrame.
 

tests-v3/test_advection.py

@@ -1,6 +1,5 @@
 import numpy as np
 import pytest
-import pandas as pd
 import xarray as xr
 
 from parcels import (
@@ -80,7 +79,7 @@ def test_analyticalAgrid():
 @pytest.mark.parametrize("v", [1, -0.3, 0, -1])
 @pytest.mark.parametrize("w", [None, 1, -0.3, 0, -1])
 @pytest.mark.parametrize("direction", [1, -1])
-def test_uniform_analytical(u, v, w, direction, tmp_parquet):
+def test_uniform_analytical(u, v, w, direction, tmp_zarrfile):
     lon = np.arange(0, 15, dtype=np.float32)
     lat = np.arange(0, 15, dtype=np.float32)
     if w is not None:
@@ -100,14 +99,14 @@ def test_uniform_analytical(u, v, w, direction, tmp_parquet):
     x0, y0, z0 = 6.1, 6.2, 20
     pset = ParticleSet(fieldset, pclass=Particle, lon=x0, lat=y0, depth=z0)
 
-    outfile = pset.ParticleFile(name=tmp_parquet, outputdt=1, chunks=(1, 1))
+    outfile = pset.ParticleFile(name=tmp_zarrfile, outputdt=1, chunks=(1, 1))
     pset.execute(AdvectionAnalytical, runtime=4, dt=direction, output_file=outfile)
     assert np.abs(pset.lon - x0 - pset.time * u) < 1e-6
     assert np.abs(pset.lat - y0 - pset.time * v) < 1e-6
     if w is not None:
         assert np.abs(pset.depth - z0 - pset.time * w) < 1e-4
 
-    ds = xr.open_zarr(tmp_parquet)
+    ds = xr.open_zarr(tmp_zarrfile)
     times = (direction * ds["time"][:]).values.astype("timedelta64[s]")[0]
     timeref = np.arange(1, 5).astype("timedelta64[s]")
     assert np.allclose(times, timeref, atol=np.timedelta64(1, "ms"))

tests-v3/test_fieldset_sampling.py

@@ -3,7 +3,6 @@
 from math import cos, pi
 
 import numpy as np
-import pandas as pd
 import pytest
 import xarray as xr
 
@@ -774,7 +773,7 @@ def test_multiple_grid_addlater_error():
     assert fail
 
 
-def test_fieldset_sampling_updating_order(tmp_parquet):
+def test_fieldset_sampling_updating_order(tmp_zarrfile):
     def calc_p(t, y, x):
         return 10 * t + x + 0.2 * y
 
@@ -806,10 +805,10 @@ def SampleP(particle, fieldset, time):  # pragma: no cover
 
     kernels = [AdvectionRK4, SampleP]
 
-    pfile = pset.ParticleFile(tmp_parquet, outputdt=1)
+    pfile = pset.ParticleFile(tmp_zarrfile, outputdt=1)
     pset.execute(kernels, endtime=1, dt=1, output_file=pfile)
 
-    ds = xr.open_zarr(tmp_parquet)
+    ds = xr.open_zarr(tmp_zarrfile)
     for t in range(len(ds["obs"])):
         for i in range(len(ds["trajectory"])):
             assert np.isclose(

tests-v3/test_particlesets.py

@@ -39,7 +39,7 @@ def test_pset_create_list_with_customvariable(fieldset):
 
 
 @pytest.mark.parametrize("restart", [True, False])
-def test_pset_create_fromparticlefile(fieldset, restart, tmp_parquet):
+def test_pset_create_fromparticlefile(fieldset, restart, tmp_zarrfile):
     lon = np.linspace(0, 1, 10, dtype=np.float32)
     lat = np.linspace(1, 0, 10, dtype=np.float32)
 
@@ -48,7 +48,7 @@ def test_pset_create_fromparticlefile(fieldset, restart, tmp_parquet):
     TestParticle = TestParticle.add_variable("p3", np.float64, to_write="once")
 
     pset = ParticleSet(fieldset, lon=lon, lat=lat, depth=[4] * len(lon), pclass=TestParticle, p3=np.arange(len(lon)))
-    pfile = pset.ParticleFile(tmp_parquet, outputdt=1)
+    pfile = pset.ParticleFile(tmp_zarrfile, outputdt=1)
 
     def Kernel(particle, fieldset, time):  # pragma: no cover
         particle.p = 2.0
@@ -58,7 +58,7 @@ def Kernel(particle, fieldset, time):  # pragma: no cover
     pset.execute(Kernel, runtime=2, dt=1, output_file=pfile)
 
     pset_new = ParticleSet.from_particlefile(
-        fieldset, pclass=TestParticle, filename=tmp_parquet, restart=restart, repeatdt=1
+        fieldset, pclass=TestParticle, filename=tmp_zarrfile, restart=restart, repeatdt=1
     )
 
     for var in ["lon", "lat", "depth", "time", "p", "p2", "p3"]: