Closed chguiterman closed 2 years ago
Hey, thanks for this PR, @chguiterman. Can you give me a simple runnable example where sea()
gives output with a bad event_years
? Fake or synthetic data is fine. I didn't see a replicable example with #187 and I want to see this thing fail like you describe.
This might also be a good case for a new unit test (which I'd be happy to write).
Thanks @brews. I don't think this requires a unit test, and the package is not failing, it's just not providing the user back the correct set of event years. This is a mistake that I made while writing the function, and is easy to fix, as described in #187. Here's what it's doing currently:
library(burnr)
data("pgm")
data("pgm_pdsi")
# `sea()` should report to the user only event years that are actually used in in the function
## Currently this doesn't happen, as described in issue #187
# Get event years
pgm_comp <- composite(pgm)
event_years <- get_event_years(pgm_comp)[[1]]
range(event_years)
#> [1] 1636 1871
# Range of pdsi series
range(as.numeric(rownames(pgm_pdsi)))
#> [1] 0 2003
# Cut it to intentionally omit events
pdsi_cut <- pgm_pdsi[1650:1850, , drop=FALSE]
# sea() will issue a warning that some event years fall outside of the time period of pdsi
pgm_sea <- sea(x=pdsi_cut, event = event_years)
#> Warning: One or more event years is outside the range of the climate series.
#> Using 17 event years: 1664 to 1842.
# But it will still provide back the original event years, not the cut version it ran in the function
pgm_sea$event_years
#> [1] 1636 1648 1664 1674 1685 1714 1724 1737 1746 1748 1752 1782 1789 1795 1798
#> [16] 1806 1818 1824 1842 1863 1871
# The function actually used only years between 1664 and 1842, as noted by the warning message
Created on 2022-03-24 by the reprex package (v0.3.0)
@brews checking in on this -- look OK to you?
@brews good point for the unit test. I'll add that to my list for a future enhancement. Thanks for merging this PR onto the next one....
In some cases,
sea()
will drop event years if they do not match a year in the climate series. The function issues a warning and prints the range of years included in the analysis. This PR follows that by only providing the event years used in the function in thesea
object created bysea()
Close #187