oceanmodeling / StormEvents

Python interfaces for observational data surrounding named storm events, born from @jreniel's ADCIRCpy
https://stormevents.readthedocs.io
GNU General Public License v3.0
22 stars 8 forks source link

Fix for compatibility with ADCIRC GAHM #76

Closed WPringle closed 1 year ago

WPringle commented 1 year ago

The ADCIRC GAHM model and the aswip pre-processor tool requires the forecast_hours to be non-zero in the fort.22 even for a best-track event (i.e., it doesn't read the datetime entries).

Added functionality to output the non-zero forecast hours when the VortexTrack.to_file() function is used with .22 filename suffix and advisory="BEST" or advisory=ATCF_Advisory.BEST is called, e.g.,

from stormevents.nhc import VortexTrack
from stormevents.nhc.atcf import ATCF_Advisory

event = VortexTrack.from_storm_name('ida', 2021)
event.to_file('Ida_besttrack.22', advisory=ATCF_Advisory.BEST, overwrite=True)
event.to_file('Ida_besttrack.22', advisory='BEST', overwrite=True)

This function call is added to one of the tests for writing Florence to file. The corresponding reference file has been updated with the non-zero forecast hour entries.

WPringle commented 1 year ago

@SorooshMani-NOAA Don't understand why tests failing on this too. deleted all output files before running them on my end and all tests passed.

SorooshMani-NOAA commented 1 year ago

@WPringle let me test on my side as well and see what I get.

Update @WPringle it seems the reason behind many of these failures is the updated version of pandas (https://pandas.pydata.org/docs/whatsnew/v2.0.0.html). To be more specific:

  • Removed deprecated Series.append(), DataFrame.append(), use concat() instead (GH35407)

and

  • Change the default argument of regex for Series.str.replace() from True to False. Additionally, a single character pat with regex=True is now treated as a regular expression instead of a string literal. (GH36695, GH24804)
SorooshMani-NOAA commented 1 year ago

@WPringle we can either take care of these pandas issues here, or we can first create a separate branch, fix the dependency issues, merge and then merge main into this branch and rerun tests.

WPringle commented 1 year ago

@WPringle we can either take care of these pandas issues here, or we can first create a separate branch, fix the dependency issues, merge and then merge main into this branch and rerun tests.

Thanks for finding the problem. Maybe let's make the separate branch and fix these pandas problems first. I am OK either way though.

SorooshMani-NOAA commented 1 year ago

OK, let me create a new branch on my machine and try to address it

SorooshMani-NOAA commented 1 year ago

@WPringle I noticed another part of the issue is actually the fact that right now the "realtime" (current year) list of storms returns empty and at least on of the tests fail due to that! I will try to find a better test for its replacement in the same branch that I'm fixing pandas dependency

SorooshMani-NOAA commented 1 year ago

@WPringle #79 fixes the pandas issue. After its merge, you can merge main to your branch and retest

SorooshMani-NOAA commented 1 year ago

@WPringle is this pull request still relevant? I think I missed merging it before. In any case it runs into the same issue as I had with those two tests failing out of no where! I found out if I keep repeating (rerunning) the test enough they succeed! Please let me know if this should be merged or is no longer relevant. Thanks

WPringle commented 1 year ago

@SorooshMani-NOAA We still need the fort.22 to fill in forecast_hours column for the best-track, so yes I think still necessary.

codecov[bot] commented 1 year ago

Codecov Report

Merging #76 (ad44fec) into main (197da88) will increase coverage by 0.14%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #76      +/-   ##
==========================================
+ Coverage   91.18%   91.33%   +0.14%     
==========================================
  Files          18       18              
  Lines        1872     1927      +55     
==========================================
+ Hits         1707     1760      +53     
- Misses        165      167       +2     
Impacted Files Coverage Δ
stormevents/stormevent.py 95.69% <ø> (ø)
stormevents/nhc/track.py 92.76% <100.00%> (+0.22%) :arrow_up:
stormevents/usgs/base.py 100.00% <100.00%> (ø)
tests/test_nhc.py 100.00% <100.00%> (ø)
tests/test_stormevent.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes