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

Fix bug in converted forcings filepath #185

Merged
merged 5 commits into from
Dec 8, 2022

Conversation

leifdenby
Copy link
Contributor

Previously the lagtraj prefix (lagtraj://) was erroneously included in the filepath of the yaml-file defining how the conversion is done.

Closes #184

Previously the lagtraj prefix (`lagtraj://`) was erroneously included
in the filepath of the yaml-file defining how the conversion is done.

Closes EUREC4A-UK#184
@leifdenby leifdenby requested a review from sjboeing November 22, 2022 14:51
@leifdenby
Copy link
Contributor Author

this is ready for review @sjboeing when you have a moment. Fixes #184

$> python -m lagtraj.forcings.create lagtraj://eurec4a_20200202_first_short --conversion lagtraj://dephy
100%|█████████████████████████████████████████████████
Wrote forcing file to `/home/leifdenby/git-repos/lagtraj/data/forcings/eurec4a_20200202_first_short.nc`
Wrote converted forcing file to `/home/leifdenby/git-repos/lagtraj/data/forcings/eurec4a_20200202_first_short.dephy.nc`
$> tree data 
data
├── domains
│   ├── eurec4a_circle_data
│   │   ├── an_model_2020-02-02.nc
│   │   ├── an_single_2020-02-02.nc
│   │   ├── fc_model_2020-02-01.nc
│   │   ├── fc_model_2020-02-02.nc
│   │   ├── fc_single_2020-02-01.nc
│   │   ├── fc_single_2020-02-02.nc
│   │   └── VERSION
│   └── eurec4a_circle.yaml
├── forcings
│   ├── eurec4a_20200202_first_short.dephy.nc
│   ├── eurec4a_20200202_first_short.dephy.yaml
│   ├── eurec4a_20200202_first_short.nc
│   └── eurec4a_20200202_first_short.yaml
└── trajectories
    ├── eurec4a_20200202_first_short.nc
    └── eurec4a_20200202_first_short.yaml

4 directories, 14 files

Copy link
Contributor

@sjboeing sjboeing left a comment

Choose a reason for hiding this comment

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

Thanks @leifdenby. May be quickest to go over this quickly tomorrow, it looks good but I need to get my head around the different subtypes, desc etc.

@leifdenby
Copy link
Contributor Author

Thanks @leifdenby. May be quickest to go over this quickly tomorrow, it looks good but I need to get my head around the different subtypes, desc etc.

The whole "types" and "subtypes" of input definitions is a bit hacky anyway. I should write a docstring for

def load_definition(
to explain what the difference is and what that function is doing. I'll do that and then it should be easier for you to review this.

@sjboeing
Copy link
Contributor

@leifdenby: are you happy to merge this in, maybe we can add some more documentation later?

@leifdenby
Copy link
Contributor Author

Tests are failing because python 3.6 is no longer supporting in Github actions CI. I'm trying to work around that in #186

I think we should talk about not supporting older python versions anyway. https://numpy.org/neps/nep-0029-deprecation_policy.html#drop-schedule

@leifdenby leifdenby merged commit 356f349 into EUREC4A-UK:master Dec 8, 2022
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.

Place converted trajectories in "trajectories" directory rather than in a "lagtraj:/" subdirectory
2 participants