[BUG] address issue #1201

[xujiboy]

PR Description

Please describe the changes proposed in the pull request:

• [BUG] address issue [BUG]import janitor causing TypeError #1201

This PR resolves #(put issue number here, and remove parentheses).

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

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

• @ericmjl

[codecov]

Codecov Report

Merging #1203 (33afc96) into dev (634093f) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev    #1203      +/-   ##
==========================================
- Coverage   98.04%   97.98%   -0.06%     
==========================================
  Files          76       76              
  Lines        3522     3524       +2     
==========================================
  Hits         3453     3453              
- Misses         69       71       +2     

[samukweku]

@Zeroto521 thoughts?

[ericmjl]

@samukweku do you remember if we are standardizing on built-in types or typing module types?

If we are using built-in types, then this PR may not be necessary. However, if we want compatibility with previous versions of python, then using built-in typing library types might be what we want to do.

[Zeroto521]

To compat py3.8 just add from __future__ import annotations at the top line.
For lower python versions, there requires the typing package.

[Zeroto521]

I more care about the reason why #1143 PR's py3.8 env didn't raise the error.

Originally posted by @Zeroto521 in #1143 (comment)

[samukweku]

@ericmjl not really. @Zeroto521 suggestion makes sense. Although I wonder why it didn't raise error as well during @Zeroto521 tests in #1143 . Wonder what is missing

[ericmjl]

@xujiboy I just resolved the conflicts here. Let's see what happens with the test coverage.

[ericmjl]

@Zeroto521 question, do you think we should adopt an official "supported Python versions" policy? We could follow something like NumPy's or pandas' official policy for supported Python versions.

[ericmjl]

Oh, @xujiboy I just realized that this PR is a duplicate of another PR: #1202.

Because this PR has been reviewed more extensively and already has the changelog line inside it, I think we should keep this one open. That said, I would like to credit both you and @joranbeasley in the changelog here for taking the effort to make a PR that solves the issue outlined in #1201.

[ericmjl]

FYI I'm not worried about the minor decrease in test coverage, and we can safely merge if we get one other approval.

[xujiboy]

FYI I'm not worried about the minor decrease in test coverage, and we can safely merge if we get one other approval.

Thank you very much @ericmjl

[Zeroto521]

If we decide to merge this pr.

There have some other places that need to be updated.

• janitor/utils.py
• janitor/functions/conditional_join.py
• janitor/functions/utils.py

Originally posted by @Zeroto521 in #1200 (comment)

[ericmjl]

I think we're good, b/c @samukweku has pushed code (which kind of constitutes an approval too!).

@Zeroto521 would you like to do the honors of merging?