Skip to content

Convert ERA5 spectra to standard direction#18

Merged
rafa-guedes merged 2 commits intowavespectra:masterfrom
JohnCHarrington:patch-1
Jun 25, 2020
Merged

Convert ERA5 spectra to standard direction#18
rafa-guedes merged 2 commits intowavespectra:masterfrom
JohnCHarrington:patch-1

Conversation

@JohnCHarrington
Copy link
Copy Markdown
Contributor

ERA5 spectra by default use the 'going-to' convention for direction. This sets the default behaviour to be converting this to the wavespectra default of 'coming-from', but leaves the option to use 'going-to' if required.

I don't know if using 'going-to' will have any affect on other library functionality, if so perhaps 'coming-from' should simply be enforced, or a warning should be raised? I put in the option primarily so that I could maintain backwards compatibility with how I've been using it the last couple of months before I noticed that the standard for this library was 'coming-from'

ERA5 spectra by default use the 'going-to' convention for direction. This sets the default behaviour to be converting this to the wavespectra default of 'coming-from', but leaves the option to use 'going-to' if required.

I don't know if using 'going-to' will have any affect on other library functionality, if so perhaps 'coming-from' should simply be enforced, or a warning should be raised?
@rafa-guedes
Copy link
Copy Markdown
Collaborator

Setting directions to "going-to" should not have any effect on other functionality within the library, however the attributes both for direction coordinates and directional integrated parameters would become inconsistent and could cause confusion. I'd be inclined to enforcing all readers returned data in the established convention. We could then perhaps define some method in the library to do the switch if we wanted to, taking care of these attributes as well, that would be independent on where the data would come from.

I'd suggest for now only apply the fix to define "coming-from" directions, and remove the option from the reader if you agree.

@JohnCHarrington
Copy link
Copy Markdown
Contributor Author

Agreed. I've changed to simply enforce "coming-from" convention.

I'm not sure how useful supporting "going-to" would be generally (I only need it for backwards compatibility for this one project which will be completed next week). If it turns out I need "going-to" for a future project as well I'll open a discussion on how best to implement it.

@rafa-guedes rafa-guedes merged commit f626a24 into wavespectra:master Jun 25, 2020
@rafa-guedes
Copy link
Copy Markdown
Collaborator

Thanks @JohnCHarrington , will release a new version including these changes soon, after merging #17.

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