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

[ENH] remove select_columns from conditional_join #1297

Merged
merged 9 commits into from
Nov 5, 2023
Merged

Conversation

samukweku
Copy link
Collaborator

@samukweku samukweku commented Oct 13, 2023

PR Description

Please describe the changes proposed in the pull request:

  • remove deprecated select_columns function from other functions/tests
  • use align instead of merge for left/right join
  • add support for timedelta dtype

This PR builds on #1288 .

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.md.
  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 samukweku self-assigned this Oct 13, 2023
@ericmjl
Copy link
Member

ericmjl commented Oct 13, 2023

@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

Merging #1297 (5d6ad68) into dev (a4f1c0a) will increase coverage by 1.45%.
Report is 4 commits behind head on dev.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev    #1297      +/-   ##
==========================================
+ Coverage   92.84%   94.30%   +1.45%     
==========================================
  Files          78       78              
  Lines        4125     4142      +17     
==========================================
+ Hits         3830     3906      +76     
+ Misses        295      236      -59     

@samukweku samukweku force-pushed the samukweku/cond_join branch from 90822c5 to 32015f8 Compare October 17, 2023 11:28
@ericmjl
Copy link
Member

ericmjl commented Nov 5, 2023

GitBot Summary of Changes

The pull request includes several changes across multiple files, primarily focusing on enhancing the conditional_join function and updating the select function usage:

  1. In CHANGELOG.md, a new enhancement is added to the list, stating that conditional_join now supports timedelta dtype.

  2. In conditional_join.py, the conditional_join function is updated to support timedelta dtype. The function now supports efficient joins on inequality operators. The function's documentation is updated to reflect these changes. Several examples are added to illustrate the new functionality.

  3. In deconcatenate_column.py, impute.py, move.py, pivot.py, utils.py, io.py, and test_clean_names.py, the usage of select_columns is replaced with select.

  4. In test_conditional_join.py, new test cases are added to verify the correct behavior of the updated conditional_join function.

  5. In xlsx_cells function in io.py, the usage of select_columns is replaced with select.

  6. In test_clean_names.py, the usage of select_columns is replaced with select in the test cases.

  7. In test_conditional_join.py, new test cases are added to verify the correct behavior of the updated conditional_join function, especially with the new support for timedelta dtype.

cc: @samukweku, please check for correctness!

@ericmjl
Copy link
Member

ericmjl commented Nov 5, 2023

@samukweku I think we're good to merge, right? I've already approved the changes, and all tests pass so far.

@samukweku
Copy link
Collaborator Author

@ericmjl yes it is ok to merge. Thanks

@ericmjl ericmjl merged commit 04fa118 into dev Nov 5, 2023
4 checks passed
@samukweku samukweku deleted the samukweku/cond_join branch November 18, 2023 02:12
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