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.

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

Update ringdown format_lmns to accept whitespace-separated string. #3454

Merged

Conversation

JulianWesterweck
Copy link
Contributor

Making the list of desired ringdown modes a string prevents issues with conflicting dimensions in the FieldArray of parameters.

@micamu
Copy link

micamu commented Sep 24, 2020

@JulianWesterweck Looks good to me! Have you tested it? I am not sure if we might need some .decode() for python3...

@JulianWesterweck
Copy link
Contributor Author

Yes, I tested it, both loading a model that uses the waveform, to make sure it is read correctly from a config file, and by just using the generator directly. So far that worked.

Copy link
Contributor

@cdcapano cdcapano 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.

@cdcapano
Copy link
Contributor

@JulianWesterweck It's minor, but could you fix the whitespace issues that code climate picked up? Other than that, @micamu do you have any objections to this being merged?

@JulianWesterweck
Copy link
Contributor Author

@cdcapano @micamu Oh, yes, I missed that one. Done.

@JulianWesterweck JulianWesterweck merged commit 5e769c3 into gwastro:master Sep 30, 2020
ArthurTolley pushed a commit to ArthurTolley/pycbc that referenced this pull request Nov 14, 2022
…wastro#3454)

* Update ringdown format_lmns to accept whitespace-separated string.

* Remove unnecessary whitespace.
OliverEdy pushed a commit to OliverEdy/pycbc that referenced this pull request Apr 3, 2023
…wastro#3454)

* Update ringdown format_lmns to accept whitespace-separated string.

* Remove unnecessary whitespace.
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