Skip to content

Improve pulse waveform validation error messagesFixed pulse validation edge case - #1058

Closed
Janvee31 wants to merge 1 commit into
pasqal-io:developfrom
Janvee31:improve-error-messages
Closed

Improve pulse waveform validation error messagesFixed pulse validation edge case#1058
Janvee31 wants to merge 1 commit into
pasqal-io:developfrom
Janvee31:improve-error-messages

Conversation

@Janvee31

@Janvee31 Janvee31 commented May 7, 2026

Copy link
Copy Markdown

Summary

Improved the validation error messages in pulse.py to make them clearer and more user-friendly.

Testing

  • Ran targeted tests:
    • pytest tests/test_pulse.py
  • Ran full test suite:
    • pytest -q

All tests passed successfully.

@a-corni

a-corni commented May 20, 2026

Copy link
Copy Markdown
Collaborator

Hello @Janvee31 ! Thanks for your contribution !

@a-corni

a-corni commented May 20, 2026

Copy link
Copy Markdown
Collaborator

Hello @Janvee31,
We have more CI tests than pytest. We notably check the "style" of the package, and would like you to run "black ." on your code to fix the "style" check. Please, have a look at our contributing section

"All samples of an amplitude waveform must be "
"greater than or equal to zero."
)
f"All samples of an amplitude waveform must be >= 0; "

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

flake8 is complaining because this f-string doesn't have any value to fill in. Delete the f to pass the tests.

"greater than or equal to zero."
)
f"All samples of an amplitude waveform must be >= 0; "
f"found invalid values in amplitude waveform: {amplitude}."

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I find the suggestion of @HGSilveri better:

Suggested change
f"found invalid values in amplitude waveform: {amplitude}."
f"found negative values in amplitude waveform: {amplitude}."

@a-corni

a-corni commented May 20, 2026

Copy link
Copy Markdown
Collaborator

This tackles partially #1057 (only for pulse waveform).

@HGSilveri

Copy link
Copy Markdown
Collaborator

Hey @Janvee31, it's been a while since this PR has been idle. Are you planning on picking it back up soon?

@a-corni

a-corni commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Closing this PR as completed by #1093

@a-corni a-corni closed this Aug 10, 2026
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.

3 participants