From 9b804ff78f6b16f5800c4470e7a1b464e38d6855 Mon Sep 17 00:00:00 2001
From: Nao Pross <np@0hm.ch>
Date: Tue, 2 Apr 2024 13:41:46 +0200
Subject: More review comments

---
 polymatrix/expression/__init__.py   | 5 +++++
 polymatrix/expression/expression.py | 3 ++-
 polymatrix/expression/init.py       | 3 +++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/polymatrix/expression/__init__.py b/polymatrix/expression/__init__.py
index a731191..6f7eae2 100644
--- a/polymatrix/expression/__init__.py
+++ b/polymatrix/expression/__init__.py
@@ -6,7 +6,9 @@ from collections.abc import Iterable
 from polymatrix.utils.getstacklines import get_stack_lines
 from polymatrix.expression.expression import init_expression, Expression
 
+# NP: Why are these functions here?
 
+# FIXME: move in the correct file
 def v_stack(
     expressions: Iterable[Expression],
 ) -> Expression:
@@ -24,12 +26,14 @@ def v_stack(
     )
 
 
+# FIXME: move in the correct file
 def h_stack(
     expressions: Iterable[Expression],
 ) -> Expression:
     return v_stack((expr.T for expr in expressions)).T
 
 
+# FIXME: move in the correct file
 def block_diag(
     expressions: tuple[Expression],
 ) -> Expression:
@@ -40,6 +44,7 @@ def block_diag(
     )
 
 
+# FIXME: move in the correct file
 def product(
     expressions: Iterable[Expression],
     degrees: tuple[int, ...] = None,
diff --git a/polymatrix/expression/expression.py b/polymatrix/expression/expression.py
index fe3957c..e8f2e42 100644
--- a/polymatrix/expression/expression.py
+++ b/polymatrix/expression/expression.py
@@ -120,7 +120,8 @@ class Expression(
             # delegate to upper level
 
             # NP: what is upper level? Class inherits from ABC and base class,
-            # NP: neither has a notion of left / right
+            # NP: neither has binary operators, also NotImplemented IIRC is only for __lt__
+            # NP: and other comparison methods and not for overloadings like __add__
             if right is None:
                 return NotImplemented
 
diff --git a/polymatrix/expression/init.py b/polymatrix/expression/init.py
index 17e7ae7..75b63b6 100644
--- a/polymatrix/expression/init.py
+++ b/polymatrix/expression/init.py
@@ -154,9 +154,12 @@ def init_from_symmetric_matrix_expr(
     )
 
 
+# NP: replace with FromDataTypes?
 DATA_TYPE = str | np.ndarray | sympy.Matrix | sympy.Expr | tuple | ExpressionBaseMixin
 
 
+# NP: this function should be split up into smaller functions, one for each "from" type
+# NP: and each "from" should be documented, explaining how it is interpreted.
 def init_from_expr_or_none(
     data: DATA_TYPE,
 ) -> ExpressionBaseMixin | None:
-- 
cgit v1.2.1