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

Make ens and time dimensions specific to variable being called. #321

Merged
merged 2 commits into from
Oct 2, 2019

Conversation

potterzot
Copy link
Contributor

Change ensemble and time indices to be set according to the variable being queried rather than be fixed.

Description

ens_idx and time_idx were previously fixed by default to

ens_idx = 1:21
time_idx = 1:65

but some variables have different lengths of time dimension. Now both indices are set based on the value range of the specific variable. In addition, ens_idx and time_idx were depreciated in favor of ens and time, which allow the user to specify the values rather than the indices. This conforms to how the other dimension values (latitude and longitude) are set.

Tests are also included to check that the new values are correctly set, that indices out of bounds return an informative error, and that indices override values if both are set (i.e. if time = c(6,12) and time_idx = 1:3, the query will return times c(6,12,18) that correspond to time indices 1:3).

Related Issue

Fixes #320

@sckott sckott added this to the v0.9 milestone Sep 23, 2019
@sckott
Copy link
Contributor

sckott commented Sep 24, 2019

thanks ! a few thigns:

  • when I installed locally, the gefs man file was updated, can you run document again to make sure that man file is updated
  • if you want people to move away from deprecated things, i think it makes sense to move to the new thing in your examples, e.g,
gefs(var, lat, lon, date = date, forecast_time = "1800", ens_idx=2:4,
      time_idx=1:8)
#> 'ens_idx' is depreciated and will be removed in future versions, please specify values (not indices) in 'ens' instead.
#> 'time_idx' is depreciated and will be removed in future versions, please specify values (not indices) in 'time' instead.

so instead change the example to use what whatever the new thing is


the last example, that is commented out, seems like it could be removed, especially since it fails, or should it stay?:

gefs(var, ens=1, time=1:12)
#> Error in round(lat, 0) : non-numeric argument to mathematical function

@sckott sckott modified the milestones: v0.9, v0.9.2 Sep 25, 2019
@sckott
Copy link
Contributor

sckott commented Oct 2, 2019

thanks, looks good

@sckott sckott merged commit 9e1bd57 into ropensci:master Oct 2, 2019
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.

gefs example failing
2 participants