code-review feedback

single-cell-data · Oct 27, 2024 · a59204c · a59204c
1 parent b843e48
commit a59204c
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 11 deletions.
diff --git a/apis/python/src/tiledbsoma/_dataframe.py b/apis/python/src/tiledbsoma/_dataframe.py
@@ -10,7 +10,6 @@
 from typing import (
     Any,
     Dict,
-    Iterable,
     List,
     Optional,
     Sequence,
@@ -507,7 +506,7 @@ def _upgrade_or_change_domain_helper(
                 f"{function_name_for_messages}: requested domain has length {len(dim_names)} but the dataframe's schema has index-column count {len(newdomain)}"
             )
 
-        if any([slot is not None and len(slot) != 2 for slot in newdomain]):  # type: ignore
+        if any([slot is not None and len(slot) != 2 for slot in newdomain]):
             raise ValueError(
                 f"{function_name_for_messages}: requested domain must have low,high pairs, or `None`, in each slot"
             )
@@ -533,7 +532,7 @@ def _upgrade_or_change_domain_helper(
                 ]
                 and new_dom is None
             ):
-                new_domain_dict[dim_name] = ("", "")
+                new_domain_dict[dim_name] = ("", "")  # type: ignore
             else:
                 new_domain_dict[dim_name] = tuple(new_dom)  # type: ignore
 

diff --git a/libtiledbsoma/src/soma/soma_array.h b/libtiledbsoma/src/soma/soma_array.h
@@ -1285,21 +1285,27 @@ class SOMAArray : public SOMAObject {
     }
 
     /**
-     * This is for SOMADataFrame.
-     * XXX comment more
+     * This is for SOMADataFrame.  While resize_soma_joinid_shape allows the
+     * user to do up the soma_joinid domain slot, without needing to specify
+     * the rest (which is the common operation for experiment-level resize)
+     * this allows the full-generality resize-every-index-column case
+     * (which only applies to variant-indexed/non-standard dataframes).
      */
-    void upgrade_domain(
+    void change_domain(
         const ArrowTable& newdomain, std::string function_name_for_messages) {
-        _set_domain_helper(newdomain, false, function_name_for_messages);
+        _set_domain_helper(newdomain, true, function_name_for_messages);
     }
 
     /**
-     * This is for SOMADataFrame.
-     * XXX comment more
+     * This is for SOMADataFrame.  While upgrade_soma_joinid_shape allows the
+     * user to do up the soma_joinid domain slot, without needing to specify
+     * the rest (which is the common operation for experiment-level resize)
+     * this allows the full-generality resize-every-index-column case
+     * (which only applies to variant-indexed/non-standard dataframes).
      */
-    void change_domain(
+    void upgrade_domain(
         const ArrowTable& newdomain, std::string function_name_for_messages) {
-        _set_domain_helper(newdomain, true, function_name_for_messages);
+        _set_domain_helper(newdomain, false, function_name_for_messages);
     }
 
    protected: