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

[enhancement] support empty array var[ [] ] #209

Closed
ryofurue opened this issue Jun 17, 2023 · 3 comments
Closed

[enhancement] support empty array var[ [] ] #209

ryofurue opened this issue Jun 17, 2023 · 3 comments

Comments

@ryofurue
Copy link

For a regular vector, v[idx] "works" even if idx == Int64[]. This is a useful property to gracefully handle edge cases. For a netCDF variable, however, v[ [] ] fails with

ERROR: LoadError: MethodError: reducing over an empty collection is not allowed; consider supplying `init` to the reducer

It would be nice if an empty array is supported. The following is a self-contained example where it would be useful.

using Dates
using NCDatasets
using Plots

t0 = DateTime(2015,5,27)
t1 = DateTime(2015,5,28)
t2 = DateTime(2015,5,29)
t3 = DateTime(2015,5,30)

function makeplots(tax, v)
  ls = findall(t -> (t1 <= t <= t2), tax)
  @show ls
  plot(tax[ls], v[ls]; xlim=(t1,t3))
  sleep(3)
  ls = findall(t -> (t2 <= t <= t3), tax)
  @show ls
  display(plot!(tax[ls], v[ls]))
end

tax  = DateTime(2015,5,28):Dates.Hour(1):DateTime(2015,5,28,23)
myvar = rand(Float64, length(tax)) # fake values

# works
makeplots(tax,myvar) # works

# create a sample
NCDataset("tmp.nc", "c") do ds
  defVar(ds, "time", tax, ("time",))
  defVar(ds, "myvar", myvar, ("time",))
end

# fails
NCDataset("tmp.nc", "r") do ds
  makeplots(ds["time"], ds["myvar"])
end
@Alexander-Barth
Copy link
Owner

Thank you for preparing the reproducer; it helps a lot to find the error quickly!
I fixed this in the master version and added a test case of it.

d79a646

@ryofurue
Copy link
Author

it helps a lot to find the error quickly

I didn't know that was an "error"! I meant to suggest that it would be nice if the empty-array behavior is emulated as an enhancement in the future because it's sometimes useful. (If I thought it was an error, I would have shortened my example, just to reproduce the error. I wanted to explain in what kind of situations the empty-array behavior may be useful.)

Anyway, thank you for your fast response!

@Alexander-Barth
Copy link
Owner

In fact, the old behavior was inconsistent (and empty range 1:0 would have worked, but not Int[]). The error message was just triggered in the checking assert code.

Thanks again!

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

No branches or pull requests

2 participants