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

allow use of sk_sg_filter by passing channel bandwidth and sampling interval #83

Merged
merged 4 commits into from
Sep 14, 2021

Conversation

mef51
Copy link
Contributor

@mef51 mef51 commented Sep 14, 2021

sk_sg_filter only uses foff and tsamp from the Your class and I wanted to apply this on waterfalls without having to instantiate this class to use it. This change allows either use case: use with your_object or optionally pass foff and tsamp instead

@devanshkv devanshkv self-requested a review September 14, 2021 04:11
@devanshkv devanshkv self-assigned this Sep 14, 2021
@devanshkv devanshkv added the enhancement New feature or request label Sep 14, 2021
@devanshkv
Copy link
Member

@mef51 thanks for the PR, this looks like a good idea! Can you please run black on the your code?

@mef51
Copy link
Contributor Author

mef51 commented Sep 14, 2021

@devanshkv sure, done

your/utils/rfi.py Outdated Show resolved Hide resolved
@devanshkv
Copy link
Member

@mef51 can you please modify the following test to include cases for your code:

your/tests/test_rfi.py

Lines 46 to 56 in 3d286d9

def test_incorrect_sigmas(fil_file):
your_object = Your(fil_file)
data = your_object.get_data(0, 1024)
with pytest.raises(ValueError):
sk_sg_filter(data, your_object, -1, 1, 1)
with pytest.raises(ValueError):
sk_sg_filter(data, your_object, 3, 15, -1)
with pytest.raises(ValueError):
sk_sg_filter(data, your_object, 0, 15, 0)

@mef51
Copy link
Contributor Author

mef51 commented Sep 14, 2021

The test was failing because of the different signature. I moved the new kwargs to the end of the arg list to preserve the signature. Also added a test case for the new kwargs

@devanshkv
Copy link
Member

Awesome! Thank you so much! We are going to make a release soon and will make sure to detail your contributions and tag you.

@devanshkv devanshkv merged commit 7b6ab05 into thepetabyteproject:master Sep 14, 2021
@mef51
Copy link
Contributor Author

mef51 commented Sep 14, 2021

Thanks! Appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants