Refactoring function name to unify the string check, isidentifier and iskeyword assert on a single function

Author: nilodna

src/parcels/_core/field.py

@@ -18,8 +18,8 @@
     AllParcelsErrorCodes,
     StatusCode,
 )
-from parcels._core.utils.string import _assert_valid_python_varname
 from parcels._core.utils.time import TimeInterval
+from parcels._core.utils.string import _assert_str_and_python_varname
 from parcels._core.uxgrid import UxGrid
 from parcels._core.xgrid import XGrid, _transpose_xfield_data_to_tzyx
 from parcels._reprs import default_repr
@@ -102,10 +102,8 @@ def __init__(
             raise ValueError(
                 f"Expected `data` to be a uxarray.UxDataArray or xarray.DataArray object, got {type(data)}."
             )
-        if not isinstance(name, str):
-            raise ValueError(f"Expected `name` to be a string, got {type(name)}.")
-        else:
-            _assert_valid_python_varname(name)
+        
+        _assert_str_and_python_varname(name)
 
         if not isinstance(grid, (UxGrid, XGrid)):
             raise ValueError(f"Expected `grid` to be a parcels UxGrid, or parcels XGrid object, got {type(grid)}.")
@@ -250,10 +248,7 @@ class VectorField:
     def __init__(
         self, name: str, U: Field, V: Field, W: Field | None = None, vector_interp_method: Callable | None = None
     ):
-        if not isinstance(name, str):
-            raise ValueError(f"Expected `name` to be a string, got {type(name)}.")
-        else:
-            _assert_valid_python_varname(name)
+        _assert_str_and_python_varname(name)
 
         self.name = name
         self.U = U

src/parcels/_core/fieldset.py

@@ -14,6 +14,7 @@
 from parcels._core.utils.string import _assert_valid_python_varname
 from parcels._core.utils.time import get_datetime_type_calendar
 from parcels._core.utils.time import is_compatible as datetime_is_compatible
+from parcels._core.utils.string import _assert_str_and_python_varname
 from parcels._core.xgrid import _DEFAULT_XGCM_KWARGS, XGrid
 from parcels._logger import logger
 from parcels._typing import Mesh
@@ -164,10 +165,7 @@ def add_constant(self, name, value):
         `Diffusion <../examples/tutorial_diffusion.ipynb>`__
         `Periodic boundaries <../examples/tutorial_periodic_boundaries.ipynb>`__
         """
-        if not isinstance(name, str):
-            raise ValueError(f"Expected `name` to be a string, got {type(name)}.")
-        else:
-            _assert_valid_python_varname(name)
+        _assert_str_and_python_varname(name)
 
         if name in self.constants:
             raise ValueError(f"FieldSet already has a constant with name '{name}'")

src/parcels/_core/particle.py

@@ -8,8 +8,8 @@
 
 from parcels._compat import _attrgetter_helper
 from parcels._core.statuscodes import StatusCode
-from parcels._core.utils.string import _assert_valid_python_varname
 from parcels._core.utils.time import TimeInterval
+from parcels._core.utils.string import _assert_str_and_python_varname
 from parcels._reprs import _format_list_items_multiline
 
 __all__ = ["KernelParticle", "Particle", "ParticleClass", "Variable"]
@@ -45,10 +45,7 @@ def __init__(
         to_write: bool | Literal["once"] = True,
         attrs: dict | None = None,
     ):
-        if not isinstance(name, str):
-            raise ValueError(f"Expected `name` to be a string, got {type(name)}.")
-        else:
-            _assert_valid_python_varname(name)
+        _assert_str_and_python_varname(name)
 
         try:
             dtype = np.dtype(dtype)

src/parcels/_core/utils/string.py

@@ -1,9 +1,28 @@
 from keyword import iskeyword, kwlist
 
+# def _assert_valid_python_varname(name):
+#     try:
+#         if name.isidentifier():
+#             if not iskeyword(name):
+#                 return
+#             raise ValueError(f"Received invalid Python variable name {name!r}: it is a reserved keyword. Avoid using the following names: {', '.join(kwlist)}")
+#         else:
+#             raise ValueError(f"Received invalid Python variable name {name!r}: not a valid identifier.")
+#     except Exception as e:
+#         raise ValueError(f"Error validating variable name {name!r}: {e}")
+    
 
-def _assert_valid_python_varname(name):
-    if name.isidentifier() and not iskeyword(name):
-        return
-    raise ValueError(
-        f"Received invalid Python variable name {name!r}. Avoid using the following names: {', '.join(kwlist)}"
-    )
+def _assert_str_and_python_varname(name):
+    if not isinstance(name, str):
+        raise TypeError(f"Expected a string for variable name, got {type(name).__name__} instead.")
+
+    if not name.isidentifier():
+        raise ValueError(
+            f"Received invalid Python variable name {name!r}: not a valid identifier. "
+            f"HINT: avoid using spaces, special characters, and starting with a number."
+        )
+    if iskeyword(name):
+        raise ValueError(
+            f"Received invalid Python variable name {name!r}: it is a reserved keyword. "
+            f"HINT: avoid using the following names: {', '.join(kwlist)}"
+        )