Fix pivot index bug
State: closed

Previously, the variable "cols" was created solely to concatenate
the lists "index" and "columns" when the "values" parameter was
left as None.  It did so in a way that it modified the variable
passed as "index" using list.extend(), affecting the caller's
namespace.

This change simply uses an anonymous list in place of the
variable "cols"

Had some trouble setting up the dev environment to build pandas from source,
so no tests checked or docs built.  However, it's fairly minor change, so hoping 
that it builds green regardless...

- [X] closes #37635 
- [X] tests added / passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry


Labels:
name: Bug
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode

Comments:
Thanks for the quick reply @arw2019! 

Quick question: I'm essentially copying `TestPivot.test_pivot()`.  The point of the bugfix is to prevent the `pivot()` method from modifying the passed argument `index`.  Should I still include the assert statements from `TestPivot.test_pivot()` to verify that the pivot itself worked, or just include an assert statement focused on the bugfix?
> Thanks for the quick reply @arw2019!
> 
> Quick question: I'm essentially copying `TestPivot.test_pivot()`. The point of the bugfix is to prevent the `pivot()` method from modifying the passed argument `index`. Should I still include the assert statements from `TestPivot.test_pivot()` to verify that the pivot itself worked, or just include an assert statement focused on the bugfix?

It's easiest if you post the test then will look. As a general rule though we like DRY code yes

Hello @Jacob-Stevens-Haas! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





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

##### Comment last updated at 2020-11-13 18:14:02 UTC
Only ``test_pivot_index_list_values_none_immutable_args`` may be a better name, since in fact the arguments must stay unchanged.
> 
> 
> Only `test_pivot_index_list_values_none_immutable_args` may be a better name, since in fact the arguments must stay unchanged.

That too was a code-review-requested name, so I'll wait for @arw2019 to comment on that suggestion

Separate issue that I may need guidance on - the test that's failing on the Azure pipeline is `TestRegistration.test_pandas_plots_register`, which asserts that a `Series.plot()` doesn't raise any warnings.  Going through relevant code for that test and `Series.plot()` now, but it's odd that the only changes between that test passing and failing were to `test_pivot.py` and `whatsnew`... no actual code changes to the package.
> > Only `test_pivot_index_list_values_none_immutable_args` may be a better name, since in fact the arguments must stay unchanged.
> 
> That too was a code-review-requested name, so I'll wait for @arw2019 to comment on that suggestion
> 

Yeah that's a better name!

> Separate issue that I may need guidance on - the test that's failing on the Azure pipeline is `TestRegistration.test_pandas_plots_register`, which asserts that a `Series.plot()` doesn't raise any warnings. Going through relevant code for that test and `Series.plot()` now, but it's odd that the only changes between that test passing and failing were to `test_pivot.py` and `whatsnew`... no actual code changes to the package.

Don't worry about it, it's unrelated (that test if flaky)
🥳 .  Now that it's passing tests, is there anything else that needs to be done?  Also, should I also try to apply the changes to the 1.1.x branch too? (1.0.x branch is stale too... is that one at end of life?)
Ok so same error to last time (`TestRegistration.test_pandas_plots_register`) on Windows37.  Errors on Windows38 are all in listed as "new" tests, mostly with memory errors... Don't believe these are a consequence of my edits, but is it something I need to solve in this PR?
thanks @Jacob-Stevens-Haas 

yeah CI has been having troubles with some windows lately.