Skip to content

Conversation

@jthorton
Copy link
Collaborator

Fixes #87

@IAlibay
Copy link
Member

IAlibay commented Sep 19, 2024

@ijpulidos I'll let you do the merging here - this is NECycling territory so you're the codeowner.

@IAlibay IAlibay requested a review from ijpulidos September 19, 2024 18:10
Copy link
Member

@IAlibay IAlibay left a comment

Choose a reason for hiding this comment

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

Just reading the initial issue - @jthorton could you add a json roundtrip test?

We have various examples of this in gufe and openfe if necessary.

@pep8speaks
Copy link

pep8speaks commented Sep 23, 2024

Hello @jthorton! 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-09-23 11:58:39 UTC

Copy link
Contributor

@ijpulidos ijpulidos left a comment

Choose a reason for hiding this comment

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

This looks good to me! Thanks.

@ijpulidos ijpulidos merged commit 75f966e into main Sep 23, 2024
@ijpulidos ijpulidos deleted the ff_type branch September 23, 2024 17:32
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.

NonEquilibriumCyclingSettings can not be round tripped to JSON

5 participants