add questions/comments

Author: majosm

pytato/stringifier.py

@@ -72,7 +72,9 @@ def __init__(self,
         # Uses the same cache for both arrays and functions
         self._cache: dict[tuple[int, int], str] = {}
 
+    # Should expr be ArrayOrNames instead of Any?
     def rec(self, expr: Any, depth: int) -> str:
+        # Why cache by ID?
         cache_key = (id(expr), depth)
         try:
             return self._cache[cache_key]

pytato/transform/__init__.py

@@ -324,6 +324,7 @@ class TransformMapper(CachedMapper[ArrayOrNames, FunctionDefinition, []]):
     implement default mapper methods; for that, see :class:`CopyMapper`.
 
     """
+    # FIXME: This inflates recursion depth
     def rec_ary(self, expr: Array) -> Array:
         res = self.rec(expr)
         assert isinstance(res, Array)
@@ -335,6 +336,7 @@ def rec_ary(self, expr: Array) -> Array:
 # {{{ TransformMapperWithExtraArgs
 
 class TransformMapperWithExtraArgs(
+            # Why multiple inheriting?
             CachedMapper[ArrayOrNames, FunctionDefinition, P],
             Mapper[ArrayOrNames, FunctionDefinition, P]
         ):
@@ -345,6 +347,7 @@ class TransformMapperWithExtraArgs(
     The logic in :class:`TransformMapper` purposely does not take the extra
     arguments to keep the cost of its each call frame low.
     """
+    # FIXME: This inflates recursion depth
     def rec_ary(self, expr: Array, *args: P.args, **kwargs: P.kwargs) -> Array:
         res = self.rec(expr, *args, **kwargs)
         assert isinstance(res, Array)