Skip to content

Conversation

gtfierro
Copy link
Collaborator

@gtfierro gtfierro commented Jul 3, 2024

Got this branch up-to-date with develop. Filing this as a draft PR so we can track progress on it

haneslinger and others added 28 commits September 12, 2023 15:32
…is going on.

# Aider chat conversation:

USER: add to the 'unify_bindings' function comments that explain what is going on

ASSISTANT: To add comments to the 'unify_bindings' function, we need to modify `buildingmotif/semantic_graph_synthesizer/bindings_utils.py`:

1. Add comments to the 'unify_bindings' function to explain what is going on.

Here is the *SEARCH/REPLACE* block:

buildingmotif/semantic_graph_synthesizer/bindings_utils.py
```python
<<<<<<< SEARCH
def unify_bindings(bindings_list: List[Bindings]) -> List[UnifiedBindings]:
    """Combine all the bindings for the same template with the same name"""
=======
def unify_bindings(bindings_list: List[Bindings]) -> List[UnifiedBindings]:
    """
    Combine all the bindings for the same template with the same name.

    This function takes a list of Bindings objects and combines those that have the same template and name.
    The result is a list of UnifiedBindings objects, where each object represents a unique combination of template and name.
    The labels and bindings of the original Bindings objects are merged into the corresponding UnifiedBindings object.
    """
>>>>>>> REPLACE
```
This change adds a more detailed explanation of what the 'unify_bindings' function does.
@gtfierro gtfierro marked this pull request as ready for review September 18, 2024 20:13
@gtfierro gtfierro requested a review from TShapinsky July 16, 2025 20:50
@gtfierro gtfierro added this to the 0.4.0 milestone Sep 3, 2025
Copy link
Member

@TShapinsky TShapinsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. If the tests pass I will merge this today.

@MatthewSteen MatthewSteen removed this from the 0.4.0 milestone Sep 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants