Fix Arrows.bounding_box() (#321)

* fix Arrows.bounding_box()

- return [[min(x), min(y), min(z)], [min(x), min(y), min(z)]]
+ return [[min(x), min(y), min(z)], [max(x), max(y), max(z)]]

* add doc strings

Author: janosh

.pre-commit-config.yaml

@@ -12,7 +12,7 @@ exclude: ^(docs/.+|.*lock.*|jupyterlab-extension/.+|.*\.(svg|js|css))$
 
 repos:
   - repo: https://github.com/charliermarsh/ruff-pre-commit
-    rev: v0.0.252
+    rev: v0.0.255
     hooks:
       - id: ruff
         args: [--fix, --ignore, D]

crystal_toolkit/core/legend.py

@@ -208,12 +208,13 @@ def generate_accessible_color_scheme_on_the_fly(
 
     @staticmethod
     def generate_categorical_color_scheme_on_the_fly(
-        site_collection: SiteCollection, site_prop_types
+        site_collection: SiteCollection, site_prop_types: dict[str, list[str]]
     ) -> dict[str, dict[str, tuple[int, int, int]]]:
-        """e.g. for Wyckoff.
+        """E.g. for Wyckoff.
 
         Args:
-            site_collection: SiteCollection
+            site_collection (SiteCollection): The sites to generate a color scheme for.
+            site_prop_types (dict[str, list[str]]): The categorical site property types.
 
         Returns: A dictionary in similar format to EL_COLORS
         """
@@ -225,9 +226,9 @@ def generate_categorical_color_scheme_on_the_fly(
             props = np.array(site_collection.site_properties[site_prop_name])
             props[props is None] = "None"
 
-            le = LabelEncoder()
-            le.fit(props)
-            transformed_props = le.transform(props)
+            label_enc = LabelEncoder()
+            label_enc.fit(props)
+            transformed_props = label_enc.transform(props)
 
             # if we have more categories than available colors,
             # arbitrarily group some categories together

crystal_toolkit/core/mpcomponent.py

@@ -224,8 +224,8 @@ def id(
         self,
         name: str = "default",
         is_kwarg: bool = False,
-        idx=False,
-        hint=None,
+        idx: bool | int = False,
+        hint: str = None,
         is_store: bool = False,
     ) -> str | dict[str, str]:
         """Generate an id from a name combined with the base id of the MPComponent itself, useful
@@ -241,9 +241,13 @@ def id(
         to parse a boolean value. In future iterations, we may be able to replace this with native
         Python type hints. The problem here is being able to specify array shape where appropriate.
 
-
         Args:
-            name: e.g. "default"
+            name (str): The name of the element, e.g. "graph", "structure". Defaults to "default".
+            is_kwarg (bool): If True, return a dict with information necessary to reconstruct
+                the keyword argument for a specific class.
+            idx (bool | int): The index to return if is_kwarg is True. Defaults to False.
+            hint (str): The type hint to return if is_kwarg is True. Defaults to None.
+            is_store (bool): If True, return the id of the store, otherwise return a dict
 
         Returns: e.g. "MPComponent_default"
         """

crystal_toolkit/core/scene.py

@@ -133,8 +133,8 @@ def bounding_box(self) -> list[list[float]]:
         """Returns the bounding box coordinates."""
         if len(self.contents) > 0:
             min_list, max_list = zip(*[p.bounding_box for p in self.contents])
-            min_x, min_y, min_z = map(min, list(zip(*min_list)))
-            max_x, max_y, max_z = map(max, list(zip(*max_list)))
+            min_x, min_y, min_z = map(min, zip(*min_list))
+            max_x, max_y, max_z = map(max, zip(*max_list))
 
             return [[min_x, min_y, min_z], [max_x, max_y, max_z]]
         else:
@@ -511,7 +511,15 @@ def key(self):
         return f"arrow_{self.color}_{self.radius}_{self.headLength}_{self.headWidth}_{self.reference}"
 
     @classmethod
-    def merge(cls, arrow_list):
+    def merge(cls, arrow_list: list[Arrows]) -> Arrows:
+        """Merge a list of arrows into a new Arrows instance.
+
+        Args:
+            arrow_list (list[Arrows]): Arrows to merge
+
+        Returns:
+            Arrows: Merged arrows
+        """
         new_positionPairs = list(
             chain.from_iterable([arrow.positionPairs for arrow in arrow_list])
         )
@@ -527,7 +535,7 @@ def merge(cls, arrow_list):
     @property
     def bounding_box(self) -> list[list[float]]:
         x, y, z = zip(*chain.from_iterable(self.positionPairs))
-        return [[min(x), min(y), min(z)], [min(x), min(y), min(z)]]
+        return [[min(x), min(y), min(z)], [max(x), max(y), max(z)]]
 
 
 @dataclass

crystal_toolkit/helpers/utils.py

@@ -515,7 +515,7 @@ def pretty_frac_format(x: float) -> str:
     return x_str
 
 
-def hook_up_fig_with_ctk_struct_viewer(
+def hook_up_fig_with_struct_viewer(
     fig: go.Figure,
     df: pd.DataFrame,
     struct_col: str = "structure",
@@ -534,16 +534,17 @@ def hook_up_fig_with_ctk_struct_viewer(
         from pymatgen.ext.matproj import MPRester
 
         # Get random structures from the Materials Project
-        mp_ids = [f"mp-{random.randint(1, 10000)}" for _ in range(100)]
-        structures = MPRester(use_document_model=False).summary.search(material_ids=mp_ids)
+        mp_ids = [f"mp-{random.randint(1, 10_000)}" for _ in range(100)]
+        docs = MPRester(use_document_model=False).summary.search(material_ids=mp_ids)
 
-        df = pd.DataFrame(structures)
+        df = pd.DataFrame(docs)
         id_col = "material_id"
 
         fig = px.scatter(df, x="nsites", y="volume", hover_name=id_col, template="plotly_white")
         app = hook_up_fig_with_ctk_struct_viewer(fig, df.set_index(id_col))
         app.run_server(port=8000)
 
+
     Args:
         fig (Figure): Plotly figure to be hooked up with the structure component. The
             figure must have hover_name set to the index of the data frame to identify

pyproject.toml

@@ -67,7 +67,7 @@ build-backend = "setuptools.build_meta"
 target-version = "py38"
 select = [
     "B",   # flake8-bugbear
-    "C4",  # flake8-comprehensions
+    "C40", # flake8-comprehensions
     "D",   # pydocstyle
     "E",   # pycodestyle
     "F",   # pyflakes