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

Update tstart and tseg when using Lightcurve.truncate() #753

Merged
merged 3 commits into from
Sep 6, 2023
Merged

Conversation

matteobachetti
Copy link
Member

Resolve #751

@codecov
Copy link

codecov bot commented Sep 1, 2023

Codecov Report

Merging #753 (70231cc) into main (5d633c1) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 70231cc differs from pull request most recent head c3790bc. Consider uploading reports for the commit c3790bc to get more accurate results

@@           Coverage Diff           @@
##             main     #753   +/-   ##
=======================================
  Coverage   97.13%   97.13%           
=======================================
  Files          42       42           
  Lines        7929     7932    +3     
=======================================
+ Hits         7702     7705    +3     
  Misses        227      227           
Files Changed Coverage Δ
stingray/lightcurve.py 97.89% <100.00%> (+<0.01%) ⬆️

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

Copy link
Collaborator

@mgullik mgullik left a comment

Choose a reason for hiding this comment

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

Hi @matteobachetti,
Thanks a lot for solving this issue!
I guess that apply_mask in the _truncate_by_index/time takes care of all the array attribute of the object light curve, however, we do not test countrate and the errors on the count and countrate. Do you think it's worth adding those checks?

More importantly, I'm not sure if the new implementation modifies the Lightcurve attribute meanrate, meancount and n (The number of data points in the light curve) after the truncation is applied. Can you check these couple of things?

Cheers!

@matteobachetti
Copy link
Member Author

@mgullik Thanks, I think I addressed your comments

@mgullik mgullik added this pull request to the merge queue Sep 6, 2023
Merged via the queue into main with commit 5b338dc Sep 6, 2023
13 checks passed
@matteobachetti matteobachetti deleted the fix_truncate branch September 6, 2023 10:14
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.

tseg and tstart not being updated after lc.truncate?
2 participants