From 397250983238fecfedac18c6f82efcd5eeec102c Mon Sep 17 00:00:00 2001
From: mferrera <matt.ferrera@gmail.com>
Date: Tue, 31 Oct 2023 10:46:15 +0100
Subject: [PATCH] CLN: Scan GridProperties keynames with roffio

When importing a roff GridProperties file the names of the properties
were scanned with C code which fails to collect keynames that being
with "NA". This change instead collects the valid keynames with
roffio. Benchmark testing with roff files of different sizes showed
that this incurs a consistent performance drop of 0.01 seconds.
---
 src/xtgeo/grid3d/_gridprops_import_roff.py | 76 ++++++++++++----------
 tests/test_grid3d/test_grid_properties.py  | 12 ++++
 2 files changed, 55 insertions(+), 33 deletions(-)

diff --git a/src/xtgeo/grid3d/_gridprops_import_roff.py b/src/xtgeo/grid3d/_gridprops_import_roff.py
index ff86c91c5..d90d417bc 100644
--- a/src/xtgeo/grid3d/_gridprops_import_roff.py
+++ b/src/xtgeo/grid3d/_gridprops_import_roff.py
@@ -1,16 +1,19 @@
-from typing import List, Union
+from __future__ import annotations
 
-from typing_extensions import Literal
+import io
+from typing import TYPE_CHECKING, List, Literal, Union
+
+import roffio
 
 import xtgeo
-from xtgeo.common.sys import _XTGeoFile
+from xtgeo.common import XTGeoDialog
 
-from . import _grid3d_utils as utils
-from .grid_property import GridProperty
+if TYPE_CHECKING:
+    from xtgeo.common.sys import _XTGeoFile
 
-xtg = xtgeo.common.XTGeoDialog()
+    from .grid_property import GridProperty
 
-logger = xtg.functionlogger(__name__)
+logger = XTGeoDialog().functionlogger(__name__)
 
 
 def import_roff_gridproperties(
@@ -18,48 +21,55 @@ def import_roff_gridproperties(
     names: Union[List[str], Literal["all"]],
     strict: bool = True,
 ) -> List[GridProperty]:
-    """Imports list of properties from a roff file.
+    """
+    Imports a list of properties from a ROFF file.
+
+    Parameters:
+        pfile:
+            Reference to the file.
+        names:
+            List of names to fetch, can also be "all" to fetch all properties.
+        strict:
+            If strict=True, will raise error if key is not found.
+            Defaults to True.
 
-    Args:
-        pfile: Reference to the file
-        names: List of names to fetch, can also be "all" to fetch all properties.
-        strict: If strict=True, will raise error if key is not found.
     Returns:
         List of GridProperty objects fetched from the ROFF file.
+
     """
-    print(pfile)
     validnames = []
+    with roffio.lazy_read(pfile.file) as contents:
+        for tagname, tagkeys in contents:
+            for keyname, values in tagkeys:
+                if tagname == "parameter" and keyname == "name":
+                    validnames.append(values)
 
-    collectdata = utils.scan_keywords(pfile, fformat="roff")
-    for item in collectdata:
-        keyname = item[0]
-        if keyname.startswith("parameter!name!"):
-            # format is 'parameter!name!FIPNUM'
-            validnames.append(keyname.split("!").pop())
+    # Rewind if this file is in memory
+    if isinstance(pfile.file, (io.BytesIO, io.StringIO)):
+        pfile.file.seek(0)
 
     usenames = []
     if names == "all":
         usenames = validnames
     else:
         for name in names:
-            if name not in validnames:
-                if strict:
-                    raise ValueError(
-                        f"Requested keyword {name} is not in ROFF file, valid "
-                        f"entries are {validnames}, set strict=False to warn instead."
-                    )
-                else:
-                    logger.warning(
-                        "Requested keyword %s is not in ROFF file. Entry will"
-                        "not be read, set strict=True to raise Error instead.",
-                        name,
-                    )
-            else:
+            if name in validnames:
                 usenames.append(name)
+                continue
+
+            if strict:
+                raise ValueError(
+                    f"Requested keyword {name} is not in ROFF file, valid "
+                    f"entries are {validnames}, set strict=False to warn instead."
+                )
+            logger.warning(
+                "Requested keyword %s is not in ROFF file. Entry will"
+                "not be read, set strict=True to raise Error instead.",
+                name,
+            )
 
     props = [
         xtgeo.gridproperty_from_file(pfile.file, fformat="roff", name=name)
         for name in usenames
     ]
-
     return props
diff --git a/tests/test_grid3d/test_grid_properties.py b/tests/test_grid3d/test_grid_properties.py
index 8075046e8..98f8750b8 100644
--- a/tests/test_grid3d/test_grid_properties.py
+++ b/tests/test_grid3d/test_grid_properties.py
@@ -122,6 +122,18 @@ def test_gridproperties_from_roff(grid_property):
     assert props.names == [grid_property.name]
 
 
+def test_gridproperties_from_roff_with_name_starting_with_na():
+    grid_property = xtgeo.GridProperty(name="NA")
+    buff = io.BytesIO()
+    grid_property.to_file(buff, fformat="roff")
+    buff.seek(0)
+    props = xtgeo.gridproperties_from_file(
+        buff, fformat="roff", names=[grid_property.name]
+    )
+
+    assert props.names == [grid_property.name]
+
+
 @given(gridproperties_elements())
 def test_gridproperties_invalid_format(grid_property):
     buff = io.BytesIO()