add ability to create a synthetic stormevent

[zacharyburnett]

No description provided.

[codecov]

Codecov Report

Merging #47 (26da757) into main (76096b8) will decrease coverage by 0.10%.
The diff coverage is 88.23%.

@@            Coverage Diff             @@
##             main      #47      +/-   ##
==========================================
- Coverage   91.10%   90.99%   -0.11%     
==========================================
  Files          18       18              
  Lines        1854     1898      +44     
==========================================
+ Hits         1689     1727      +38     
- Misses        165      171       +6     

Impacted Files	Coverage Δ
stormevents/stormevent.py	93.68% <80.00%> (-2.49%)	⬇️
tests/test_stormevent.py	98.70% <100.00%> (+0.20%)	⬆️

[SorooshMani-NOAA]

As a follow up to the discussion at #46 (comment):
I feel kind of uneasy with this. I love having something that can interface with a perturber object (e.g. storm.perturb(perturber, *args, **kwargs)) but the synthetic input argument kind of kills most of the internal checks here, which makes me a bit nervous.

If this functionality is mainly added for testing, I think we might be better off using packages such as unittest. See: https://docs.python.org/3/library/unittest.mock.html

What do you think?

[zacharyburnett]

As a follow up to the discussion at #46 (comment): I feel kind of uneasy with this. I love having something that can interface with a perturber object (e.g. storm.perturb(perturber, *args, **kwargs)) but the synthetic input argument kind of kills most of the internal checks here, which makes me a bit nervous.

If this functionality is mainly added for testing, I think we might be better off using packages such as unittest. See: https://docs.python.org/3/library/unittest.mock.html

What do you think?

I am unsure how else to tell a StormEvent object whether it is synthetic or not (so it doesn't try to go and validate itself from the Internet). I think it is important to explicitly define whether a storm is real, for the user. What internal checks does it kill?

There is a use case for building synthetic storms, and I think it should be possible for the user to do that; perhaps, however, it might be better to make it a separate class, like the following:

     Storm (abstract base class)
           /       \
          /         \
SyntheticStorm   StormEvent (real storm)    

[SorooshMani-NOAA]

I really glanced at the code updates, but in general I noticed in most of the places where there were any checks, it would just say if synthetic just ignore. They might have all been checks with real atcf storms which doesn't really need to be applied to synthetic storms.

Having the class hierarchy would help with this. Doing so will help in making sure the necessary checks are separated from real storm checks, which makes the code clearer as well.

The main thing that was bothering me was that to me it seemed like the synthetic storm was added only for testing, which could be done by mock packages. But for perturbation of real storms having synthetic storm makes sense, in those cases however, we still need some checking against real data (e.g. for dates, calculating deviation from real track, etc.)