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

Metadata for JRA55 #286

Open
wants to merge 49 commits into
base: main
Choose a base branch
from
Open

Metadata for JRA55 #286

wants to merge 49 commits into from

Conversation

simone-silvestri
Copy link
Collaborator

@simone-silvestri simone-silvestri commented Dec 5, 2024

This PR is an initial proposal to generalize ECCOMetadata to Metadata and rework the JRA55 module to use Metadata. In this way, we can have different JRA55 versions (repeat year and multiple year) and we can define a download_dataset function to download the dataset independently of using JRA55 as we can do for ECCO

This PR also removes the ability to generate a JRA55FieldTimeSeries directly interpolated on the ocean grid, since we need to interpolate anyways when we compute fluxes

closes #182

@glwagner
Copy link
Member

glwagner commented Dec 5, 2024

I think we should get #251 and #284 merged first, do you want to help with those? Otherwise we will have conflicts.

@simone-silvestri
Copy link
Collaborator Author

sounds good

@simone-silvestri simone-silvestri marked this pull request as ready for review February 12, 2025 14:39
@simone-silvestri
Copy link
Collaborator Author

This should be ready to merge once the tests pass. However, we can probably merge #294 first and this one later. They should not conflict since this has nothing to do with the kernels but just with the organization of the data.

However, this PR should be ready to review

@simone-silvestri
Copy link
Collaborator Author

Another change I made is that JRA55FieldTimeSeries does not accept time_indices anymore, but dates (as ECCO does). I was thinking this is more in line with the metadata implementation and uniformizes the interface between the different datasets. I can revert it to how it was previously though

@simone-silvestri
Copy link
Collaborator Author

Tests should pass so this is ready to merge / review

@simone-silvestri simone-silvestri requested review from navidcy and glwagner and removed request for navidcy February 14, 2025 14:39
Copy link

codecov bot commented Feb 15, 2025

Codecov Report

Attention: Patch coverage is 0% with 208 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (5a3d6b4) to head (16d2b17).

Files with missing lines Patch % Lines
src/DataWrangling/JRA55/JRA55_field_time_series.jl 0.00% 110 Missing ⚠️
src/DataWrangling/metadata.jl 0.00% 31 Missing ⚠️
src/DataWrangling/JRA55/JRA55_metadata.jl 0.00% 29 Missing ⚠️
...DataWrangling/JRA55/JRA55_prescribed_atmosphere.jl 0.00% 29 Missing ⚠️
src/DataWrangling/ECCO/ECCO_metadata.jl 0.00% 4 Missing ⚠️
src/DataWrangling/ECCO/ECCO_restoring.jl 0.00% 3 Missing ⚠️
src/DataWrangling/ECCO/ECCO_mask.jl 0.00% 2 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main    #286   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files         35      38    +3     
  Lines       2281    2227   -54     
=====================================
+ Misses      2281    2227   -54     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

Refactoring JRA55
2 participants