Skip to content

Conversation

@mstechly
Copy link
Contributor

@mstechly mstechly commented Nov 5, 2024

Description

Adds support for repeated structures.

There's a couple of things left:

  • Documentation
  • Fixing style issues

Please verify that you have completed the following steps

  • I have self-reviewed my code.
  • I have included test cases validating introduced feature/fix.
  • I have updated documentation.

@mstechly mstechly requested a review from dexter2206 November 5, 2024 10:39
@cla-bot cla-bot bot added the cla-signed label Nov 5, 2024
@mstechly mstechly marked this pull request as ready for review November 6, 2024 11:34
Copy link
Contributor

@dexter2206 dexter2206 left a comment

Choose a reason for hiding this comment

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

Looks good, I requested only minor changes in docstrings and examples.

count: N
sequence:
type: closed_form
sum: "(2 ^ (X + 1) - 1)"
Copy link
Contributor

Choose a reason for hiding this comment

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

question: In some places we use ** to denote exponentiation, while in other places like here we use ^. WDYT about unifying it so that every exponentiation is represented with the same symbol?

Personally, I'd like it to be ** because it is more pythonic, and ^ is meant to mean xor in Python. We support both notations in Bartiq, but users familiar with Python might find the current state of the examples confusing - if there are examples of ** and examples of ^, a reasonable conclusion is that those two operations are diferent, right?

On the other hand, QREF is (programming)language-agnostic and we don't discuss language of arithmetic expressions in the documentation - it is therefore subject to interpretation by client libraries.

What's your stance on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, I think ** is better choice.
It's language agnostic, but reality is that most people will use that with Python anyway, assuming pythonic syntax is not bad.

mstechly and others added 3 commits November 7, 2024 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants