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

Determining the frequency of the peak(s) #55

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

HeshamElsaman
Copy link

Determining the frequency of the peak(s)

Linting

  • The code lints

Contents

  • Added a function the determines the frequency of the peak(s) taking the median of the power spectrum as a threshold and its unit test
  • Files changed: preparation.py, test_preparation.py, requirements.txt

Reviewer

@HeshamElsaman HeshamElsaman added the Algorithms/Math For anything that is related to Algorithm Design or Math label Nov 8, 2024
@HeshamElsaman HeshamElsaman changed the title H--HW5.1 Determining the frequency of the peak(s) Nov 8, 2024
@HeshamElsaman
Copy link
Author

@zbpetersbuf here, I edited the power spectrum function that you created

@HeshamElsaman
Copy link
Author

@zbpetersbuf and also the function to calculate frequencies

@HeshamElsaman
Copy link
Author

@zbpetersbuf I guess the only problem was that they always returned (None) because the data did not pass the equally-spacing check

@HeshamElsaman
Copy link
Author

@zbpetersbuf what I did was to allow some tolerance in the spacing since the spacing is in float-point

@HeshamElsaman
Copy link
Author

@zbpetersbuf maybe you can check the rest of the functions for the same issue, because I only edited those two functions

@HeshamElsaman
Copy link
Author

@zbpetersbuf my other issue was in realizing the type of the data needed to be entered to those functions

@HeshamElsaman
Copy link
Author

@zbpetersbuf my suggestions is that you can add more details to the docstrings to emphasize the usage of pandas.Series and that the data needs to have timestamps

@zbpetersbuf
Copy link
Contributor

this error tollerance is correct good job

@laserlab laserlab requested a review from zbpetersbuf November 8, 2024 20:51
@laserlab
Copy link
Contributor

laserlab commented Nov 8, 2024

@zbpetersbuf Can you review this since you already looked into this?

@zbpetersbuf
Copy link
Contributor

shouldnt some one else do the review so they can get points for it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Algorithms/Math For anything that is related to Algorithm Design or Math
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants