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

Refactor: Made NVAS optional kwargs keyword only #2656

Merged
merged 2 commits into from
Feb 8, 2023

Conversation

nkphysics
Copy link
Member

Refactored nvas to make optional kwargs keyword only.
Addresses nvas on list for #1746

Tests (remote, local, and docs) should be passing.

@codecov
Copy link

codecov bot commented Feb 4, 2023

Codecov Report

Merging #2656 (957aca4) into main (4b29a35) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2656   +/-   ##
=======================================
  Coverage   68.87%   68.87%           
=======================================
  Files         304      304           
  Lines       22529    22529           
=======================================
  Hits        15516    15516           
  Misses       7013     7013           
Impacted Files Coverage Δ
astroquery/nvas/core.py 92.59% <100.00%> (ø)

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

@nkphysics nkphysics marked this pull request as ready for review February 4, 2023 00:12
@nkphysics
Copy link
Member Author

If this PR works for you'll, I'll try a grouping of whats still listed in #1746 in another PR next week.

@bsipocz bsipocz added the nvas label Feb 8, 2023
@bsipocz bsipocz added this to the v0.4.7 milestone Feb 8, 2023
@bsipocz bsipocz merged commit 766a4fc into astropy:main Feb 8, 2023
@bsipocz
Copy link
Member

bsipocz commented Feb 8, 2023

Thanks @nkphysics!

Having one PR for the rest (or multiple) of the remaining modules would be awesome.

@nkphysics nkphysics deleted the nvas-kwarg-refactor branch February 8, 2023 18:05
@bsipocz bsipocz mentioned this pull request Feb 16, 2023
62 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants