Skip to content

[ENH] variable args for complete #857

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

Merged
merged 19 commits into from
Aug 19, 2021
Merged

Conversation

samukweku
Copy link
Collaborator

@samukweku samukweku commented Jul 26, 2021

PR Description

Please describe the changes proposed in the pull request:

  • Uses variable args (*args) for the complete function.

This PR improves complete function API.

PR Checklist

Please ensure that you have done the following:

  1. PR in from a fork off your branch. Do not PR from <your_username>:dev, but rather from <your_username>:<feature-branch_name>.
  1. If you're not on the contributors list, add yourself to AUTHORS.rst.
  1. Add a line to CHANGELOG.md under the latest version header (i.e. the one that is "on deck") describing the contribution.
    • Do use some discretion here; if there are multiple PRs that are related, keep them in a single line.

Automatic checks

There will be automatic checks run on the PR. These include:

  • Building a preview of the docs on Netlify
  • Automatically linting the code
  • Making sure the code is documented
  • Making sure that all tests are passed
  • Making sure that code coverage doesn't go down.

Relevant Reviewers

Please tag maintainers to review.

@samukweku
Copy link
Collaborator Author

noticed that the docs are not rendering properly (links not working etc). @ericmjl @hectormz might need a refresher on what to check

@codecov
Copy link

codecov bot commented Jul 26, 2021

Codecov Report

Merging #857 (fd8e7bc) into dev (c62333e) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #857      +/-   ##
==========================================
- Coverage   94.92%   94.92%   -0.01%     
==========================================
  Files          19       19              
  Lines        2090     2089       -1     
==========================================
- Hits         1984     1983       -1     
  Misses        106      106              

@samukweku samukweku mentioned this pull request Aug 14, 2021
3 tasks
Copy link
Member

@ericmjl ericmjl left a comment

Choose a reason for hiding this comment

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

Wonderful work, @samukweku! I love the direction you've taken here, making the API much more uniform with variable args 😄

@ericmjl
Copy link
Member

ericmjl commented Aug 19, 2021

@samukweku could you resolve the conflicts and then ping back here? I think the PR has been open long enough that we should merge. 😄

@ericmjl ericmjl merged commit 7189d6d into pyjanitor-devs:dev Aug 19, 2021
@ericmjl
Copy link
Member

ericmjl commented Aug 19, 2021

Thanks @samukweku!

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