Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

added check if spectrum is real #27

Merged
merged 6 commits into from
Sep 21, 2024
Merged

added check if spectrum is real #27

merged 6 commits into from
Sep 21, 2024

Conversation

stroblme
Copy link
Member

@stroblme stroblme commented Aug 26, 2024 •
edited
Loading

This PR resolves #25 by adding

  • assertion to check that coefficients are real valued
  • test for coefficients

Additionally, the PR introduces

  • __main__ calls for all test scripts

Melvin Strobl added 3 commits August 26, 2024 09:42
added check if spectrum is real
4d51a7d 
Signed-off-by: Melvin Strobl <[email protected]>
added coefficient test
0ce2cdf 
Signed-off-by: Melvin Strobl <[email protected]>
added main calls
290c21b 
Signed-off-by: Melvin Strobl <[email protected]>
@pep8speaks
Copy link

pep8speaks commented Aug 26, 2024 •
edited
Loading

Hello @stroblme! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-08-27 07:58:28 UTC

@stroblme stroblme requested a review from majafranz August 26, 2024 08:24
Melvin Strobl added 3 commits August 26, 2024 10:26
pep
5a76a7e 
Signed-off-by: Melvin Strobl <[email protected]>
added kwargs and force mean value flag
89c6291 
Signed-off-by: Melvin Strobl <[email protected]>
Revert "added kwargs and force mean value flag"
96c0d46 
This reverts commit 89c6291.
majafranz
majafranz approved these changes Sep 21, 2024
View reviewed changes
Copy link
Collaborator

@majafranz majafranz 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 to me. Thanks a lot (and sry for the delay in merging)

@majafranz majafranz merged commit d6a3b3b into main Sep 21, 2024
1 check passed
@majafranz majafranz deleted the real-valued-spectrum branch November 15, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@majafranz majafranz majafranz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FR] Add assertion to only allow real valued spectrum
3 participants
@stroblme @pep8speaks @majafranz