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

Bayesian Excess Variance (Bexvar) in Stingray - GSoC'22 project #664

Merged
merged 26 commits into from
Aug 24, 2022

Conversation

mihirtripathi97
Copy link
Contributor

This is the base code for implementing Bayesian Excess Variance (bexvar) in Stingray (#582).

This project is part of Google Summer of Code 2022, a brief summary of the project can be found here.

@pep8speaks
Copy link

pep8speaks commented Jul 10, 2022

Hello @mihirtripathi97! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 55:20: E261 at least two spaces before inline comment
Line 57:10: E261 at least two spaces before inline comment
Line 75:90: E502 the backslash is redundant between brackets
Line 76:9: E128 continuation line under-indented for visual indent
Line 112:48: E203 whitespace before ','
Line 112:66: E502 the backslash is redundant between brackets
Line 113:13: E128 continuation line under-indented for visual indent
Line 118:1: E302 expected 2 blank lines, found 1
Line 148:17: E221 multiple spaces before operator
Line 159:71: E502 the backslash is redundant between brackets
Line 160:9: E128 continuation line under-indented for visual indent
Line 168:49: E251 unexpected spaces around keyword / parameter equals
Line 168:51: E251 unexpected spaces around keyword / parameter equals
Line 168:66: E251 unexpected spaces around keyword / parameter equals
Line 168:68: E251 unexpected spaces around keyword / parameter equals
Line 168:83: E251 unexpected spaces around keyword / parameter equals
Line 168:85: E251 unexpected spaces around keyword / parameter equals
Line 184:101: E501 line too long (102 > 100 characters)
Line 187:1: W293 blank line contains whitespace
Line 200:79: W291 trailing whitespace
Line 214:44: E203 whitespace before ','
Line 251:91: W291 trailing whitespace
Line 265:56: E231 missing whitespace after ','
Line 268:48: E231 missing whitespace after ','
Line 269:44: E231 missing whitespace after ','
Line 271:43: E231 missing whitespace after ','

Line 13:1: E302 expected 2 blank lines, found 1

Comment last updated at 2022-07-22 12:25:47 UTC

 Created a new function bexvar_from_table() to read data from astropy table and call bexvar().
Updated docstrings of functions bexvar() and bexvar_from_table (). 
Added some basic docstrings for other functions in code.
Added ultranest in list of optional dependancies in setup.cfg
@codecov
Copy link

codecov bot commented Jul 20, 2022

Codecov Report

Merging #664 (cad13a3) into main (1bd574a) will decrease coverage by 7.34%.
The diff coverage is 45.55%.

❗ Current head cad13a3 differs from pull request most recent head 9f48fbb. Consider uploading reports for the commit 9f48fbb to get more accurate results

@@            Coverage Diff             @@
##             main     #664      +/-   ##
==========================================
- Coverage   97.12%   89.78%   -7.35%     
==========================================
  Files          41       42       +1     
  Lines        7554     7644      +90     
==========================================
- Hits         7337     6863     -474     
- Misses        217      781     +564     
Impacted Files Coverage Δ
stingray/bexvar.py 45.55% <45.55%> (ø)
stingray/largememory.py 10.99% <0.00%> (-84.05%) ⬇️
stingray/pulse/overlapandsave/test_ols.py 68.62% <0.00%> (-31.38%) ⬇️
stingray/pulse/__init__.py 76.92% <0.00%> (-23.08%) ⬇️
stingray/base.py 77.15% <0.00%> (-19.83%) ⬇️
stingray/modeling/parameterestimation.py 67.29% <0.00%> (-18.75%) ⬇️
stingray/pulse/pulsar.py 89.43% <0.00%> (-9.16%) ⬇️
stingray/gti.py 94.91% <0.00%> (-4.33%) ⬇️
stingray/utils.py 95.19% <0.00%> (-3.81%) ⬇️
stingray/powerspectrum.py 95.65% <0.00%> (-3.77%) ⬇️
... and 6 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@matteobachetti matteobachetti self-requested a review July 28, 2022 08:15
@mihirtripathi97 mihirtripathi97 marked this pull request as ready for review July 29, 2022 11:03
Copy link
Member

@matteobachetti matteobachetti left a comment

Choose a reason for hiding this comment

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

Good job! Some small requests, they should be quick to implement.

stingray/bexvar.py Outdated Show resolved Hide resolved
stingray/bexvar.py Outdated Show resolved Hide resolved
stingray/bexvar.py Show resolved Hide resolved
stingray/tests/test_bexvar.py Outdated Show resolved Hide resolved
Fixed test case that checks if weight warning arises.
Copy link
Member

@matteobachetti matteobachetti left a comment

Choose a reason for hiding this comment

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

A few additions, looking at the lines that are not covered by tests

stingray/bexvar.py Show resolved Hide resolved
stingray/bexvar.py Show resolved Hide resolved
Added a condition to check if src_counts are either zero or positive integers and raise a warning if they are not.
Added new test for lines not covered by previous tests.
Update in condition that checks if `src_counts` are zero or positive integers.
Copy link
Member

@matteobachetti matteobachetti left a comment

Choose a reason for hiding this comment

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

Great job @mihirtripathi97!
It's approved, but I have very minor request: please run a last check of the consistency of docstrings. I just notice that, e.g. Iterable is sometimes capitalized, sometimes not.

@matteobachetti matteobachetti merged commit f7c981d into StingraySoftware:main Aug 24, 2022
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