Skip to content

A bug fixed by correcting missing parentheses#3

Merged
sum33it merged 1 commit intogwastro:masterfrom
sum33it:bug_fix1
Dec 17, 2025
Merged

A bug fixed by correcting missing parentheses#3
sum33it merged 1 commit intogwastro:masterfrom
sum33it:bug_fix1

Conversation

@sum33it
Copy link
Member

@sum33it sum33it commented Dec 9, 2025

This PR fixes a bug in the code. The parenthesis were missing at couple of places.

@sum33it sum33it requested a review from MaxMelching December 9, 2025 19:19
@sum33it sum33it changed the title A bug fixed vy correcting missing paranthesis A bug fixed by correcting missing parentheses Dec 9, 2025
Copy link
Collaborator

@MaxMelching MaxMelching left a comment

Choose a reason for hiding this comment

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

This certainly fixes the bug, but couldn't we also just remove the ( right before dict_waveform_modification? Then we do not need the closing parentheses in lines 102, 246 (or the ones that are added, does not matter). And I do not see any reason why Python would want them there

@sum33it sum33it merged commit ecdeba3 into gwastro:master Dec 17, 2025
6 checks passed
@spxiwh
Copy link

spxiwh commented Dec 17, 2025

@sum33it Please could you implement @MaxMelching 's suggestion here. Having additional, unnecessary extra parentheses impacts on code readability.

PyCBC aims to use black's (https://pypi.org/project/black/) formatting. If you remove the parentheses and reformat (this entire code) with black it would make things more readable.

@sum33it
Copy link
Member Author

sum33it commented Dec 17, 2025

Hi @spxiwh Thanks for pointing this out. I will do it in next PR.

@MaxMelching
Copy link
Collaborator

@spxiwh I like the black suggestion. Is there any specific config setup that PyCBC uses? Looking through the pyproject.toml and some other files, I couldn't find one, so I guess you are using the black defaults?

@spxiwh
Copy link

spxiwh commented Dec 17, 2025

We just use the default settings ... Though having a properly defined config within the repository ... and actually running it on a bunch of older code would be good!

This is described here (https://github.com/gwastro/pycbc/wiki/How-to-satisfy-CodeClimate) ... I agree that this is not an ideal place for this to live!

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