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

Feature/rolling spectrums #191

Merged
merged 24 commits into from
Apr 29, 2022
Merged

Feature/rolling spectrums #191

merged 24 commits into from
Apr 29, 2022

Conversation

S-Hanly
Copy link
Contributor

@S-Hanly S-Hanly commented Apr 7, 2022

Lots in this!

  • Added rolling_fft to endaq.calc.fft
  • Added rolling_psd to endaq.calc.psd
  • Added rolling_shock_spectrum to endaq.calc.shock
  • Allowed endaq.calc.shock.shock_spectrum to calculate the freqs for you (instead of requiring other function call)
  • Added spectrum_over_time to endaq.plot.plots
  • Fixed octave_spectrogram

This would close / address: #189, #35, #156,

This specific branch is tested in this colab with lots of examples

@codecov-commenter
Copy link

codecov-commenter commented Apr 7, 2022

Codecov Report

Merging #191 (760f41d) into development (9e720c4) will increase coverage by 0.02%.
The diff coverage is 52.65%.

@@               Coverage Diff               @@
##           development     #191      +/-   ##
===============================================
+ Coverage        61.82%   61.84%   +0.02%     
===============================================
  Files               34       34              
  Lines             2656     2857     +201     
===============================================
+ Hits              1642     1767     +125     
- Misses            1014     1090      +76     
Impacted Files Coverage Δ
endaq/calc/shock.py 77.01% <9.52%> (-10.13%) ⬇️
endaq/calc/psd.py 67.87% <36.78%> (-21.75%) ⬇️
endaq/calc/fft.py 63.12% <39.13%> (-1.59%) ⬇️
endaq/plot/plots.py 40.75% <76.54%> (+24.52%) ⬆️
endaq/calc/utils.py 86.66% <100.00%> (+4.05%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Copy link
Contributor

@CFlaniganMide CFlaniganMide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not going to point out all the same stuff as the other PR (yet), just calling out a few things that are unique to this one.

endaq/calc/fft.py Outdated Show resolved Hide resolved
endaq/calc/fft.py Outdated Show resolved Hide resolved
endaq/calc/shock.py Show resolved Hide resolved
endaq/plot/plots.py Show resolved Hide resolved
@S-Hanly
Copy link
Contributor Author

S-Hanly commented Apr 12, 2022

@CFlaniganMide let me know when you want to review this, I have the unit tests in there now and am curious how well written you think they are?

@CFlaniganMide CFlaniganMide self-requested a review April 26, 2022 12:56
tests/plot/test_plots.py Outdated Show resolved Hide resolved
tests/calc/test_fft.py Outdated Show resolved Hide resolved
tests/calc/test_fft.py Outdated Show resolved Hide resolved
tests/calc/test_fft.py Outdated Show resolved Hide resolved
tests/calc/test_fft.py Outdated Show resolved Hide resolved
endaq/calc/psd.py Outdated Show resolved Hide resolved
@S-Hanly
Copy link
Contributor Author

S-Hanly commented Apr 28, 2022

@CFlaniganMide I think I've done everything now correctly, but let me know of course if not!

@CFlaniganMide CFlaniganMide self-requested a review April 29, 2022 14:08
Copy link
Contributor

@CFlaniganMide CFlaniganMide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, also requesting some issues get made to flesh out some of the unit tests at a later date but they're good enough as-is.

@S-Hanly
Copy link
Contributor Author

S-Hanly commented Apr 29, 2022

Added! #192

@S-Hanly S-Hanly merged commit c45970e into development Apr 29, 2022
@S-Hanly S-Hanly deleted the feature/rolling_spectrums branch April 29, 2022 14:51
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.

3 participants