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

Remove optional defaults from repr for SWAN #116

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pbranson
Copy link
Member

@pbranson pbranson commented Sep 12, 2024

The REPR for the INPGRID command is currently including an option NHEDVEC always when this command is only relevant for vector based inputs (WIND, CURRENT). I think that when an attribute equal to the default that swan has we shouldnt write it to the REPR to prevent inadvertently introducing invalid combinations of parameters that swan doesnt like.

cc @alsonathif

@pbranson pbranson changed the title Nhedvec Remove optional defaults from repr for SWAN Sep 12, 2024
@pbranson pbranson marked this pull request as draft September 13, 2024 00:59
@rafa-guedes
Copy link
Collaborator

That makes sense @pbranson. I normally get the INPGRID cmd from the Data classes / interfaces which construct the cmd from the swan accessor methods rather than the components - so I hadn't noticed this one. Your changes look good - but this PR also includes the changes from #115 as well - is that intentional?

@pbranson
Copy link
Member Author

Ah, no that was accidental and my inadequate git skills. I guess I just need to rebase to the trunk after 115 was merged?

@rafa-guedes
Copy link
Collaborator

We could also just merge this one. I'm happy with your proposal in #115, perhaps it would be good just adding some test to demonstrate the behaviour you are implementing there.

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.

2 participants