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

[TST] Add test docstrings to ml, biology, chemistry modules #649

Merged
merged 6 commits into from
Apr 1, 2020

Conversation

hectormz
Copy link
Collaborator

PR Description

Please describe the changes proposed in the pull request:

  • Adds docstrings to tests in following modules:
    • ml
    • biology
    • chemistry

I did not write any of the original functions or tests, and am open to feedback for better descriptions.

This PR partially addresses #306.

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.rst 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.

Quick Check

To do a very quick check that everything is correct, follow these steps below:

  • Run the command make check from pyjanitor's top-level directory. This will automatically run:
    • black formatting
    • flake8 checking
    • running the test suite
    • docs build

Once done, please check off the check-box above.

If make check does not work for you, you can execute the commands listed in the Makefile individually.

Relevant Reviewers

Please tag maintainers to review.

@codecov
Copy link

codecov bot commented Mar 31, 2020

Codecov Report

Merging #649 into dev will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev     #649   +/-   ##
=======================================
  Coverage   92.36%   92.36%           
=======================================
  Files          16       16           
  Lines         524      524           
=======================================
  Hits          484      484           
  Misses         40       40           

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.

Only one minor change requested, @hectormz. As always, you've done a wonderful job here 😄.

tests/chemistry/test_maccs_keys_fingerprint.py Outdated Show resolved Hide resolved
tests/chemistry/test_morgan_fingerprint.py Show resolved Hide resolved
Co-Authored-By: Eric Ma <ericmjl@users.noreply.github.com>
@hectormz
Copy link
Collaborator Author

@ericmjl thanks for the feedback, just updated it!

@ericmjl ericmjl merged commit c1fd682 into pyjanitor-devs:dev Apr 1, 2020
@ericmjl
Copy link
Member

ericmjl commented Apr 1, 2020

Thanks @hectormz!

@hectormz
Copy link
Collaborator Author

hectormz commented Apr 1, 2020

More to follow...eventually...

@ericmjl
Copy link
Member

ericmjl commented Apr 1, 2020

No pressure! 😄

@hectormz
Copy link
Collaborator Author

hectormz commented Apr 1, 2020

Bitesize. It's actually also a really nice to gain some more familiarity with parts of the codebase I'm not familiar with. So good for new and old contributors.

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