Repository: pysal/momepy

Issue:
BUG: get_node_id does not support string IDs
If `edge_id` column is using string IDs, this line will fail as numpy supports only numeric dtypes.

https://github.com/pysal/momepy/blob/78c1b244f3acef60d77cbce98e3426c755b3033d/momepy/elements.py#L807C16-L807C24

We need to change it to `pd.isna`.

Discussion:
jGaboardi: Seeing now that the line isn't being hit by any tests...
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/pysal/momepy/pull/522?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal) Report
> Merging [#522](https://app.codecov.io/gh/pysal/momepy/pull/522?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal) (8a59fa8) into [main](https://app.codecov.io/gh/pysal/momepy/commit/891156e0607b2a626c572692a4c5b2aa445b767a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal) (891156e) will **not change** coverage.
> The diff coverage is `50.0%`.

<details><summary>Additional details and impacted files</summary>


[![Impacted file tree graph](https://app.codecov.io/gh/pysal/momepy/pull/522/graphs/tree.svg?width=650&height=150&src=pr&token=VNn0WR5JWT&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal)](https://app.codecov.io/gh/pysal/momepy/pull/522?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal)

```diff
@@          Coverage Diff          @@
##            main    #522   +/-   ##
=====================================
  Coverage   97.5%   97.5%           
=====================================
  Files         24      24           
  Lines       4115    4115           
=====================================
  Hits        4011    4011           
  Misses       104     104           
```

| [Files](https://app.codecov.io/gh/pysal/momepy/pull/522?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal) | Coverage Δ | |
|---|---|---|
| [momepy/elements.py](https://app.codecov.io/gh/pysal/momepy/pull/522?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bW9tZXB5L2VsZW1lbnRzLnB5) | `96.6% <50.0%> (ø)` | |

</details>
martinfleis: It is technically covering a corner case that should not happen when everything is as expected.

We hit it when we don't have a street network edge attached to a building footprint, which happens only if a building is really far away from any street. But we could possibly simulate that by using `momepy.get_network_id` with a small `min_size` (that API and implementation there are bad...)
jGaboardi: > It is technically covering a corner case that should not happen when everything is as expected.
> 
> We hit it when we don't have a street network edge attached to a building footprint, which happens only if a building is really far away from any street. But we could possibly simulate that by using `momepy.get_network_id` with a small `min_size` (that API and implementation there are bad...)

Working on creating a synthetic test case now that simply converts some edge IDs in the `bubenec` streets to NaN.
jGaboardi: Passing locally and hitting the offending line.

Patch:
diff --git a/momepy/elements.py b/momepy/elements.py
index 0d56795..53727c6 100644
--- a/momepy/elements.py
+++ b/momepy/elements.py
@@ -804,8 +804,8 @@ def get_node_id(
             total=objects.shape[0],
             disable=not verbose,
         ):
-            if np.isnan(eid):
-                results_list.append(np.nan)
+            if pd.isna(eid):
+                results_list.append(pd.NA)
             else:
                 edge = edges.loc[eid]
                 start_id = edge.node_start


Test patch:
diff --git a/momepy/tests/test_elements.py b/momepy/tests/test_elements.py
index aa7791d..f4ebdac 100644
--- a/momepy/tests/test_elements.py
+++ b/momepy/tests/test_elements.py
@@ -3,6 +3,7 @@ from random import shuffle
 
 import geopandas as gpd
 import numpy as np
+import pandas as pd
 import pytest
 from geopandas.testing import assert_geodataframe_equal
 from packaging.version import Version
@@ -154,8 +155,17 @@ class TestElements:
         self.df_buildings["nID"] = mm.get_network_id(
             self.df_buildings, self.df_streets, "nID"
         )
-        ids = mm.get_node_id(self.df_buildings, nodes, edges, "nodeID", "nID")
-        assert not ids.isna().any()
+        ids1 = mm.get_node_id(self.df_buildings, nodes, edges, "nodeID", "nID")
+        assert not ids1.isna().any()
+
+        # test for NaNs within `object` nIDs column
+        edges["nID"] = edges["nID"].astype(str)
+        _df_buildings = self.df_buildings.copy()
+        _df_buildings["nID"] = _df_buildings["nID"].astype(str)
+        _df_buildings.loc[[0, 1], "nID"] = pd.NA
+        ids2 = mm.get_node_id(_df_buildings, nodes, edges, "nodeID", "nID")
+        assert ids2.isna().sum() == 2
+        np.testing.assert_array_equal(ids2[ids2.isna()].index, [0, 1])
 
     def test_get_node_id_ratio(self):
         nx = mm.gdf_to_nx(self.df_streets)