Skip to content

Conversation

@CuylenE
Copy link
Contributor

@CuylenE CuylenE commented May 25, 2020

Tests still to be added.
I replaced the method to join a multiindex & a single index because outer joins didn't include data from the single index. The new method is similar to a join between 2 multiindexes.
I'm not sure what the keep_order parameter was, but it was never used and in other _join-methods it didn't exist either. So I suggest removing it all together.

@pep8speaks
Copy link

pep8speaks commented May 25, 2020

Hello @CuylenE! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-05-25 23:04:33 UTC
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

please always add test first

@CuylenE
Copy link
Contributor Author

CuylenE commented May 25, 2020

please always add test first

Test added. Though I'm not sure why non-related test are failing.

Copy link
Contributor

@arw2019 arw2019 left a comment

Choose a reason for hiding this comment

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

@CuylenE are you interested in picking this back up? If yes merge master, resolve the conflicts and we can take it from there

@arw2019
Copy link
Contributor

arw2019 commented Nov 29, 2020

Closing for now as stale. @CuylenE ping us if you'd like to pick this up and we'll reopen

@arw2019 arw2019 closed this Nov 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

5 participants