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

Updates to spectral() behavior #540

Merged
merged 5 commits into from
May 4, 2024
Merged

Updates to spectral() behavior #540

merged 5 commits into from
May 4, 2024

Conversation

CommonClimate
Copy link
Collaborator

Addresses #536
Summary of changes:

  • the freq_method kwarg has been replaced by freq, which can be str or array. If a string, will use make_freq_vector with the specified frequency-generating method. If an array, this will be passed directly to the spectral method.

  • the default frequency method for Series.spectral() is no longer 'log' ; it is now None, and the code assigns freq depending on method. (e.g. method=wwz triggers freq=log, but method = lomb_scargle triggers freq = lomb_scargle ). Of course, that can be overridden easily, and users can still force freq = log with any method if they so choose. Evenly-spaced methods (e.g. MTM) are not affected.

  • make_freq_vector() now has a new argument, rayleigh_bound, which forces the lowest frequency to be the Rayleigh frequency (f_R) if set to True, which is the default. I am not passing this at the API level, because in my mind, if people are going to screw around with a custom frequency vector, they can generate it themselves (using make_freq_vector for instance) and pass it to the method as freq. We’ve historically done this through settings. I don’t mind making this an extra argument but I don’t really see the point.

  • freq_vector_lomb_scargle()used to have default ofac=4, hifac=1.0. I’ve now reduced the default hifac to 0.95 to avoid the weirdness at the Nyquist frequency. Open to other suggestions.

  • freq_vector_log used to assign a default fmin that was 2*f_R. I’m not sure where that came from (naturally it was not explained anywhere), and it created discrepancies with the other methods. I’ve now aligned that so fmin = f_R if None is provided to the method.

  • docstrings have been updated, but I have not rebuilt the docs, so I expect some minor bugs. Similarly, the spectral PyleoTutorial will need a major overhaul, which I'm happy to do once this is approved.

@khider khider merged commit e02830e into master May 4, 2024
1 check passed
@khider khider deleted the freq_vector branch May 4, 2024 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

2 participants