-
-
Notifications
You must be signed in to change notification settings - Fork 396
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
Make utils and utis/tap kwargs keyword only #2690
Conversation
a27ed43
to
f0c761c
Compare
Codecov Report
@@ Coverage Diff @@
## main #2690 +/- ##
=======================================
Coverage ? 65.73%
=======================================
Files ? 233
Lines ? 17841
Branches ? 0
=======================================
Hits ? 11728
Misses ? 6113
Partials ? 0
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@@ -12,7 +12,7 @@ class MockResponse: | |||
A mocked/non-remote version of `astroquery.query.AstroResponse` | |||
""" | |||
|
|||
def __init__(self, content=None, url=None, headers={}, content_type=None, | |||
def __init__(self, content=None, *, url=None, headers={}, content_type=None, |
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.
This content
kwarg is the only kwarg that I've had to leave to be positional since the majority of the packages' testing uses it positionally. Otherwise most of the packages' testing would need to be refactored.
I can't seem to figure out where these 2 additional misses in utils/tap/ are that codecov is reporting. |
9446561
to
395d38d
Compare
Ok, I figured out why codecov was mad... Should be ok now. |
This should be ready for review. |
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.
One minor comment, and that this should be also added to the changelog.
docs/esa/hubble/hubble.rst
Outdated
>>> esahubble.get_postcard(observation_id="j6fl25s4q", calibration_level="RAW", resolution=256, filename="raw_postcard_for_j6fl25s4q.jpg") # doctest: +IGNORE_OUTPUT | ||
>>> esahubble.get_postcard("j6fl25s4q", calibration_level="RAW", resolution=256, filename="raw_postcard_for_j6fl25s4q.jpg") # doctest: +IGNORE_OUTPUT |
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.
I would keep this one as is.
Thanks @nkphysics! |
This PR is to address #1746 by making kwargs keyword only for utils and utils/tap.
As I expected, there are many packages that use some of these kwargs in a positional manner. So these changes have broken many packages. I started working though the packages to make the necessary changes so everything not listed in #2634 works again.
Like PRs I've done before here, I'm doing this pretty modularly so I figured I'd create the PR to get some input from yall on whether or not you want me to squash all the fixes to the different packages since in some cases its a 1-3 line change.