Set up PyDough subcollection nodes in AST module

[knassre-bodo]

Adding more collections to the AST module for subcollection relationships (both regulars & compounds).

• A new Collection AST class SubCollection which is a subclass of TableCollection (inherits the same lazy scheme of property evaluation) with an associated property & parent collection AST node (the ancestor)
• A new Collection AST class CompoundSubCollection which is a subclass of SubCollection for compound relationships. It fully unravels the compound relationship & any sub-compounds within it into a sequence of SubCollection invocations, with each inherited property having its location in the sequence & original name at that location defined. The key to this unravelling is a recursive method populate_subcollection_chain which is only called as part of the lazy evaluation of properties.
• A way to build subcollections via the node builder.
• Modifying the lazy evaluation of TableCollection to now handle subcollections & compounds (still lazy, to avoid infinite cycles)
• A new TestInfo class SubCollectionInfo (works for both regular & compounds)

The handling of BACK references, including to inherited properties, will be handled in another PR. For now, the inherited properties are accessed "normally", but the information is available to be able to derive where they came from.

[njriasan]

LGTM! See my comments and possible typos I found, but overall this looks good.

[njriasan]

To avoid this issue of being too verbose I think we could split this into declaration and assignment. Something like

_subcollection_property: SubcollectionRelationshipMetadata

def __init__(
         self,
         parent: PyDoughCollectionAST,
         subcollection_property: SubcollectionRelationshipMetadata,
     ):
         _subcollection_property = subcollection_property

Not urgent but might help with readability, especially since some of these assignments are split onto multiple lines.

[knassre-bodo]

That would make it a class attribute rather than an instance-level one (unless we use @dataclass). I think the best I can do is:

def __init__(
         self,
         parent: PyDoughCollectionAST,
         subcollection_property: SubcollectionRelationshipMetadata,
     ):
         self._subcollection_property: SubcollectionRelationshipMetadata
         self._subcollection_property = subcollection_property

[knassre-bodo]

UGH... I just realized that doing the way I suggested causes problems with the type annotation propagation if I do it to other things, like this:

        self._collection: CollectionMetadata = collection
        self._ancestor: PyDoughCollectionAST = ancestor
        self._predecessor: PyDoughCollectionAST | None = predecessor
        self._properties: Dict[str, Tuple[int | None, PyDoughAST]] | None = None

[knassre-bodo]

I think the best we can do maybe is just split it up sometimes :(

[njriasan]

Okay this is a very minor issue. No need to waste time on this.

[njriasan]

This looks like a typo.

[knassre-bodo]

This was a hacky trick to ensure that the lazy evaluation of self.properties was completed before this property was accessed, since the first time self.properties is evaluated it populates this property.

Do you have a suggestion of an alternative way to do this that isn't so funky?

[njriasan]

Feel free to push back on this, but I think I would remove _inheritance_sources all together. If you have a common function that "computes" all the properties I would just have that return a map/json and do something like return self.compute_properties["inheritance_sources"]

Then I would change @property to @cached_property since what you are doing is basically just caching the output, so it better for readability that it happens behind the scenes.

[knassre-bodo]

The problem is that it is only derivable as part of the datastructures that are calculated during the derivation of the entirety of self.properties; I don't think it can be calculated standalone without repeating all of the recursive computations.

[njriasan]

This looks like a typo.