Skip to content

removing old redundant code that yields SettingWithCopyWarning, remove depracated inplace, and fix hanging tests #88

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

Merged

Conversation

tekendratimsina
Copy link
Contributor

@tekendratimsina tekendratimsina commented Jun 28, 2022

This hack, which was created due to the bug during that time, has now been fixed. So i am assuming it is better to remove it rather than fixing code(util.py:226) that is giving SettingWithCopyWarning. Also, inplace parameter in pandas.Categorical.add_categories is deprecated and thus removed.
Also, some fix on tests !

@tekendratimsina tekendratimsina force-pushed the fix_redundant_code_that_gives_warning branch from 60f5c1c to c049961 Compare June 29, 2022 08:12
@tekendratimsina tekendratimsina force-pushed the fix_redundant_code_that_gives_warning branch from c049961 to 520ca19 Compare June 29, 2022 08:42
@tekendratimsina
Copy link
Contributor Author

tekendratimsina commented Jun 29, 2022

@aiguofer pytest seems to not working as intended for test_oauth_first_time and test_oauth_first_time_no_save tests from Test_get_creds which on my initial check is due to this commit where authorization flow remains hanging.
Could you please have a look at it whenever feasible !

Update: I think i manage to fix it on latest commit !

@tekendratimsina tekendratimsina changed the title removing old redundant code that is causing SettingWithCopyWarning removing old redundant code that yields SettingWithCopyWarning, remove depracated inplace, and fix hanging tests Jun 29, 2022
@aiguofer
Copy link
Owner

awesome, thanks so much for your contribution! Yeah I think removing that fix is probably good, doubt many people are still using a known bad version of pandas... and if so they can upgrade. And thanks for fixing the test!

@aiguofer aiguofer merged commit 4cb7216 into aiguofer:master Jun 29, 2022
@aiguofer
Copy link
Owner

released in 3.2.1

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