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
Remove Non-functional Overlay Args from Skyview #2979
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2979 +/- ##
==========================================
- Coverage 66.83% 66.81% -0.02%
==========================================
Files 237 237
Lines 18327 18323 -4
==========================================
- Hits 12248 12243 -5
- Misses 6079 6080 +1 ☔ View full report in Codecov by Sentry. |
I believe the failing doctests are unrelated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As noted in the referenced issue: I'm in favor of removing these w/o deprecation because they never did anything.
81cdbad
to
add48b9
Compare
Rebased at the request of @bsipocz! I believe the test failures are the same as in main: https://github.com/astropy/astroquery/actions/runs/8964265630/job/24664194149#step:5:640 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, let's have this in as is without a deprecation.
If we get any user reports during the 0.4.8 dev cycle about the exception they now receive we can roll back and do it properly with a deprecation and a warning.
CI failure is unrelated and should be addressed separately. Thank you @duytnguyendtn, and welcome on board! |
As discussed in #2976,
lut
,grid
, andgridlabel
arguments only apply for JPEG output from Skyview, though Astroquery only returns FITS objects. These values are effectively non-functional for Astroquery. This PR removes those arguments as arguments from the relevant Skyview methods and updates the docstring accordingly. Skyview tests and codestyle checks were checked and passing.Changelog will be updated once this PR goes live and a PR number is generated.
Thanks!
Duy Nguyen (NASA HEASARC)
Fixes #2976