[ENH] Add also

[sauln]

PR Description

Please describe the changes proposed in the pull request:

• Adds also method as discussed in Add an also method akin to Kotlins also  #731
• Adds tests for also
• Adds docstring for also

This PR resolves #731.

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

2. If you're not on the contributors list, add yourself to AUTHORS.rst.

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

Code Changes

If you are adding code changes, please ensure the following:

• Ensure that you have added tests.
• Run all tests ($ pytest .) locally on your machine.
  • Check to ensure that test coverage covers the lines of code that you have added.
  • Ensure that all tests pass.

Documentation Changes

If you are adding documentation changes, please ensure the following:

• Build the docs locally.
• View the docs to check that it renders correctly.

Relevant Reviewers

Please tag maintainers to review.

• @ericmjl

[sauln]

Locally, everything passed except the tests with Spark. Also, when I build the docs there is are a ton of changes. Do you want the new files in docs/reference committed?

[ericmjl]

Very thoughtful setup on the tests, @sauln! Thanks for the effort put in 😄. I'm only requesting some changes regarding docstrings.

The spark tests might fail locally, but that shouldn't worry you. As long as they don't fail on the CI, which is the ultimate arbiter of whether the tests passed or not, that's cool with me.

Speaking of the CI, it was flake8 that was failing. Some of my suggestions below should help.

Other than that, it's just adding docstrings to the tests. I noticed I had trouble deciphering some tests that others had submitted, so I've started asking for docstrings on tests. (Hynek Schwalak has a great blog post on this.) Please modify my suggestions if they're phrased incorrectly!

[ericmjl]

@sauln I just realized I forgot to respond to one of your questions:

Also, when I build the docs there is are a ton of changes. Do you want the new files in docs/reference committed?

No need to worry about these files, as they are built automatically on readthedocs and for the backup deploy to github pages!

[ericmjl]

Everything else looks great. Thanks @sauln!

[sauln]

Awesome! Thank you @ericmjl for the thorough feedback and interest in this new feature :D I'm excited to make use of it in my own day-to-day.

[sauln]

Alright, it looked like I was using some features of unittest only available in Python 3.8.. I've reworked the tests with mocks to reflect the 3.6 API (https://docs.python.org/3.6/library/unittest.mock.html#calls-as-tuples).

@ericmjl it looks like DeepSource doesn't like the tuple unpacking the Python docs recommend. Is this something we can silence, or would you rather I rework it to avoid the bug risk?

[codecov]

Codecov Report

Merging #735 into dev will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev     #735   +/-   ##
=======================================
  Coverage   93.53%   93.53%           
=======================================
  Files          17       17           
  Lines         634      634           
=======================================
  Hits          593      593           
  Misses         41       41           

[ericmjl]

@sauln hmm, I thought that the tuple unpacking inside the tests would be fine. Is there a way to rework those lines such that the bug risk is removed? If not, don't fret it, just add the skipcq: piece on those lines.

[ericmjl]

@sauln thanks for the changes! I’m going to invite review from at least one more core dev before we merge (or wait 2 days, whichever is earlier). This prevents the situation where only one person, i.e. myself, knows what’s going on in the codebase.

[sauln]

Sweet, thank you!