openearth / oceanwaves-python

Toolbox to handle ocean waves datasets
http://oceanwaves.readthedocs.io
MIT License
64 stars 39 forks source link

promote input dtypes to avoid ValueError: Integers to negative in… #19

Closed caiostringari closed 6 years ago

caiostringari commented 6 years ago

Promote input parameters dtypes to avoid ValueError: Integers to negative integer powers are not allowed.

This bug seems to be caused when you do something like this:

Hs = np.arange(1,10,1)

E = jonswap(f, Hs[0], Tp)

hoonhout commented 6 years ago

Hi Caio,

It has been a while! Thanks for contributing again to the oceanwaves toolbox.

There are still some issues with this pull request. See the test results at CircleCI.

You assume that the input parameters are numpy arrays, but that is often not the case. They are usually simple floats. Therefore your construct with param.astype(...) doesn't work. See also the inline docs of the jonswap spectrum.

I think we can make this function accept numpy arrays, but there should be some check on the input being an array. Can you add that in order to make the tests succeed?

And can you add a test in tests/test_spectral.py that fails without your modification and succeeds with your modification, including a one line description?

And if all tests are successful, can you add a line to docs/whatsnew.rst under unreleased -> bug fixes describing your fix and acknowledging yourself?

Thanks a lot, Bas

caiostringari commented 6 years ago

Hi Bas,

I will have a deeper look at this issue some point this week. It seemed to work fine on my won built when I tested it last Friday. I did not realize that there was automatic testing, awesome btw! I am working on some SWASH simulations that may use hundreds of parametric spectrum as input, so need JONSWAP to work with arrays.

Cheers,

Caio

codecov-io commented 6 years ago

Codecov Report

Merging #19 into master will decrease coverage by 0.44%. The diff coverage is 60.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #19      +/-   ##
==========================================
- Coverage   76.97%   76.53%   -0.45%     
==========================================
  Files           9        9              
  Lines        1164     1189      +25     
  Branches      239      248       +9     
==========================================
+ Hits          896      910      +14     
- Misses        163      168       +5     
- Partials      105      111       +6
Impacted Files Coverage Δ
oceanwaves/spectral.py 81.03% <60.71%> (-18.97%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c58009c...6b35421. Read the comment docs.

caiostringari commented 6 years ago

Hi Bas,

I update the code and all the tests passed. I am not sure if I followed all the procedures correctly, I've never used these tools before.

Cheers,

Caio