Skip to content

Access the correct variables in BPMHScalarAggregator and BPMVScalarAggregators#81

Merged
JeanLucPons merged 4 commits intomainfrom
bugfix/bpm-aggregators
Nov 20, 2025
Merged

Access the correct variables in BPMHScalarAggregator and BPMVScalarAggregators#81
JeanLucPons merged 4 commits intomainfrom
bugfix/bpm-aggregators

Conversation

@kparasch
Copy link
Copy Markdown
Contributor

@kparasch kparasch commented Nov 19, 2025

Because they are inheriting a private variable, their names are mangled. It is ugly to access the variables like this. I would like advice how you think we could do this better.

Changes:

  • Add test that originally fails when accessing bpm aggregators' h and v properties.
  • Move __lattice and __refpts of BPMScalarAggregator to public namespace.
  • Reverse BPMHScalarAggregator, BPMVScalarAggregator as they were doing the opposite.

Solves #80 .

…gregator. Their names are mangled, as is done when inheriting a private variable
@kparasch kparasch requested a review from JeanLucPons November 19, 2025 17:31
@JeanLucPons
Copy link
Copy Markdown
Contributor

I agree that it is not very user friendly and that it is not supposed to happen. I would suggest to either use a get_lattice() function or to move the field to public in the super class.

@gubaidulinvadim gubaidulinvadim linked an issue Nov 20, 2025 that may be closed by this pull request
@kparasch
Copy link
Copy Markdown
Contributor Author

I moved them to public namespace and added test. This is ready for final review and merge @JeanLucPons .

Copy link
Copy Markdown
Contributor

@JeanLucPons JeanLucPons left a comment

Choose a reason for hiding this comment

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

Nice work Kostas !
Thanks.
We will have to discuss soon about your Interface, your get_many() and your config file ;)

@JeanLucPons JeanLucPons merged commit 94af97f into main Nov 20, 2025
2 checks passed
@gubaidulinvadim gubaidulinvadim deleted the bugfix/bpm-aggregators branch November 25, 2025 10:45
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.

BPMArray is not linked correctly with simulator

2 participants