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

New plot type: ccf_freq #104

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open

Conversation

heavelock
Copy link

I needed a new plot type, similar to ccftime but in frequency domain. Check this out.
ccf_time_github

- Reduces length of some lines
- Uses only one text delimiter
- Adds some spaces around commas
Additionally some sormatting of msnoise.py script
- Removes old docs
- Renames fft method
- Changes to inputs
- Adds additional TODOS
Adds first two full test cases, two tests at least are pending
Creates a test suite to test multiple classes.
This is just and additional feature that was introduced by other branch.
I didn't want to merge it thus I just added those few lines
@ThomasLecocq
Copy link
Member

Hehe, really cool :) I actually coded that one live during the Cargèse Workshop and never finalized it !

@codecov
Copy link

codecov bot commented Oct 24, 2017

Codecov Report

Merging #104 into master will decrease coverage by 0.85%.
The diff coverage is 87.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #104      +/-   ##
==========================================
- Coverage   74.28%   73.42%   -0.86%     
==========================================
  Files          17       17              
  Lines        2477     2480       +3     
==========================================
- Hits         1840     1821      -19     
- Misses        637      659      +22
Impacted Files Coverage Δ
msnoise/s000installer.py 56.6% <ø> (-0.81%) ⬇️
msnoise/api.py 61.07% <87.5%> (-2.52%) ⬇️
msnoise/move2obspy.py 74.13% <0%> (-1.15%) ⬇️
msnoise/s06compute_dtt.py 78.21% <0%> (-1.12%) ⬇️
msnoise/s03compute_cc.py 71.62% <0%> (-0.34%) ⬇️

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 7f0f0ee...1b7973e. Read the comment docs.

@heavelock
Copy link
Author

Cool! That's good that this plot can be useful for somebody else than me :)

I've updated docs and added an example image, for me it looks like almost complete. What do you think?

@ThomasLecocq
Copy link
Member

Could you try to do only one thing per PR ? I mean, not refactoring PEP8 , adding starttime/endtime and adding a new feature at once ? It's tricky to understand as a "single PR".

to remain consistent with the ccftime I'd call the new plot ccffreq or ccfspec without underscore.

@heavelock
Copy link
Author

heavelock commented Nov 2, 2017

Sure, I will try to be more concentrated in future PRs :) If you prefer not to include the starttime/endtime args I can remove them.

Yup, I will rename it.

@ThomasLecocq
Copy link
Member

will be closed once #140 pass

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.

None yet

2 participants