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

Prevent duplicate sub-interval data labels #120

Merged
merged 3 commits into from
Jun 21, 2024

Conversation

bmorris3
Copy link
Contributor

@bmorris3 bmorris3 commented Jun 18, 2024

If you load Kepler data from Quarter 10 with a data_label = "Target in Q10", the parser will happily add another "Q10" to the end of the data label 🐱. This PR checks if the sub-interval for any mission is already in the data label, and only adds one if none is found.

@bmorris3 bmorris3 modified the milestones: 0.4.0, 0.5.0 Jun 18, 2024
@bmorris3 bmorris3 force-pushed the no-repeats-in-data-label branch from db6f2db to 72927e3 Compare June 18, 2024 20:35
@bmorris3
Copy link
Contributor Author

It looks like lighkurve is calling functions in oktopus, which depends on autograd, which is no longer maintained. This was causing a test failure.

@bmorris3 bmorris3 force-pushed the no-repeats-in-data-label branch from fa33ad6 to 09c9d24 Compare June 18, 2024 20:55
Copy link

codecov bot commented Jun 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.33%. Comparing base (7b78df5) to head (09c9d24).
Report is 23 commits behind head on main.

Current head 09c9d24 differs from pull request most recent head 5cbb875

Please upload reports for the commit 5cbb875 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #120      +/-   ##
==========================================
- Coverage   93.92%   91.33%   -2.60%     
==========================================
  Files          37       41       +4     
  Lines        1598     2055     +457     
==========================================
+ Hits         1501     1877     +376     
- Misses         97      178      +81     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bmorris3 bmorris3 marked this pull request as ready for review June 18, 2024 21:10
@bmorris3 bmorris3 requested a review from kecnry June 18, 2024 21:10
@kecnry
Copy link
Member

kecnry commented Jun 20, 2024

The other option would be to just not append at all if manually passing a data label - can you think of a case where providing a data label would still want the quarter/whatever automatically appended?

Copy link
Member

@kecnry kecnry left a comment

Choose a reason for hiding this comment

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

Thanks!

@kecnry kecnry merged commit 2236ade into spacetelescope:main Jun 21, 2024
11 checks passed
@@ -24,6 +24,7 @@ dependencies = [
# to devdeps in tox.ini
"jdaviz>=3.10.2,<3.11",
"lightkurve>=2.4.1",
"numpy<2",
Copy link
Member

Choose a reason for hiding this comment

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

backported in #126

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.

2 participants