-
Notifications
You must be signed in to change notification settings - Fork 5
Add Var.push_dim #354
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
base: main
Are you sure you want to change the base?
Add Var.push_dim #354
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,6 +81,7 @@ Var.find_corresponding_dim_name_in_var | |
| Var.dim_units | ||
| Var.range_dim | ||
| Base.permutedims(var::OutputVar, perm) | ||
| Var.push_dim | ||
| Var.reordered_as | ||
|
Comment on lines
83
to
85
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am also not sure if this belongs in ClimaAnalysis extension in ClimaCalibrate or ClimaAnalysis. |
||
| Var.resampled_as | ||
| Var.convert_units | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -75,7 +75,8 @@ export OutputVar, | |||||||||||||||
| reverse_dim, | ||||||||||||||||
| reverse_dim!, | ||||||||||||||||
| remake, | ||||||||||||||||
| permutedims | ||||||||||||||||
| permutedims, | ||||||||||||||||
| push_dim | ||||||||||||||||
|
|
||||||||||||||||
| """ | ||||||||||||||||
| Representing an output variable | ||||||||||||||||
|
|
@@ -2649,6 +2650,38 @@ function Base.cat(vars::OutputVar...; dims::String) | |||||||||||||||
| ) | ||||||||||||||||
| end | ||||||||||||||||
|
|
||||||||||||||||
| """ | ||||||||||||||||
| push_dim(var::OutputVar, dim_name, dim_value, dim_attrib_pairs...) | ||||||||||||||||
|
|
||||||||||||||||
| Push a singleton dimension to the end of the dimensions in `var`. | ||||||||||||||||
|
|
||||||||||||||||
| If you want to change the order of the dimensions, then call `permutedims` after | ||||||||||||||||
| this function. | ||||||||||||||||
| """ | ||||||||||||||||
| function push_dim(var::OutputVar, dim_name, dim_value, dim_attrib_pairs...;) | ||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a huge fan of this function name, but I also can't think of a better name. Maybe something like |
||||||||||||||||
| dims = deepcopy(var.dims) | ||||||||||||||||
|
Comment on lines
+2661
to
+2662
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't work properly if
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this because dims is a dict with values that are arrays of floats? Could you just make a new dict and the loop over the old one and add the keys/values?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you don't plan on supporting this, it might be good to catch the case and give an informative error |
||||||||||||||||
| dim_attribs = deepcopy(var.dim_attributes) | ||||||||||||||||
| data = deepcopy(var.data) | ||||||||||||||||
|
|
||||||||||||||||
| conventional_dim_names = conventional_dim_name.(keys(dims)) | ||||||||||||||||
| if conventional_dim_name(dim_name) in conventional_dim_names | ||||||||||||||||
| error( | ||||||||||||||||
| "Dimension ($dim_name) cannot be added as it already corresponds to an existing dimension ($conventional_dim_names)", | ||||||||||||||||
| ) | ||||||||||||||||
|
Comment on lines
+2668
to
+2670
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Minor, but it would be good to print the conflicting dimension for the user. |
||||||||||||||||
| end | ||||||||||||||||
|
|
||||||||||||||||
| dims[dim_name] = [dim_value] | ||||||||||||||||
| dim_attrib = | ||||||||||||||||
| isempty(dim_attrib_pairs) ? valtype(dim_attribs)() : | ||||||||||||||||
| Dict(dim_attrib_pairs) | ||||||||||||||||
| dim_attribs[dim_name] = dim_attrib | ||||||||||||||||
|
|
||||||||||||||||
| new_dims = (size(data)..., 1) | ||||||||||||||||
| data = reshape(data, new_dims) | ||||||||||||||||
|
|
||||||||||||||||
| return remake(var, dims = dims, data = data, dim_attributes = dim_attribs) | ||||||||||||||||
| end | ||||||||||||||||
|
|
||||||||||||||||
| """ | ||||||||||||||||
| Base.show(io::IO, var::OutputVar) | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3544,6 +3544,46 @@ end | |
| ) | ||
| end | ||
|
|
||
| @testset "Push dim" begin | ||
| lon = [-60.0, 45.0] | ||
| var = | ||
| TemplateVar() |> | ||
| add_dim("lon", lon, units = "degrees") |> | ||
| add_attribs(long_name = "hi") |> | ||
| initialize | ||
|
|
||
| time_var = ClimaAnalysis.push_dim(var, "time", 2.0) | ||
|
|
||
| @test time_var.attributes == var.attributes | ||
| @test time_var.dims["lon"] == var.dims["lon"] | ||
| @test time_var.dims["time"] == [2.0] | ||
| @test length(time_var.dim_attributes) == 2 | ||
| @test time_var.dim_attributes["lon"] == var.dim_attributes["lon"] | ||
| @test time_var.dim_attributes["time"] == Dict{String, String}() | ||
| @test isequal(vec(time_var.data), vec(var.data)) | ||
| @test size(time_var.data) == (2, 1) | ||
|
|
||
| lat_var = ClimaAnalysis.push_dim( | ||
| var, | ||
| "lat", | ||
| 0.0, | ||
| "units" => "s", | ||
| "is_lat" => "yes", | ||
| ) | ||
| @test lat_var.attributes == var.attributes | ||
| @test lat_var.dims["lon"] == var.dims["lon"] | ||
| @test lat_var.dims["lat"] == [0.0] | ||
| @test length(lat_var.dim_attributes) == 2 | ||
| @test lat_var.dim_attributes["lon"] == var.dim_attributes["lon"] | ||
| @test lat_var.dim_attributes["lat"] == | ||
| Dict("units" => "s", "is_lat" => "yes") | ||
| @test isequal(vec(lat_var.data), vec(var.data)) | ||
| @test size(lat_var.data) == (2, 1) | ||
|
|
||
| # Error handling | ||
| @test_throws ErrorException ClimaAnalysis.push_dim(var, "lon", 42.0) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a test for adding a dimension with a different name but same conventional name? For example, adding a "longitude" dimension since the "lon" dimension exists already: |
||
| end | ||
|
|
||
| @testset "Show" begin | ||
| lat = collect(range(-89.5, 89.5, 180)) | ||
| lon = collect(range(-179.5, 179.5, 360)) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.