Stop concat from attempting to sort mismatched columns by default
Preserve column order upon concatenation to obey
least astonishment principle.

Allow old behavior to be enabled by adding a boolean switch to
concat and DataFrame.append, mismatch_sort, which is by default disabled.  

- [x] Close #4588
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Hello @brycepg! Thanks for updating the PR.

































Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on May 01, 2018 at 00:20 Hours UTC # [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/20613?src=pr&el=h1) Report
> Merging [#20613](https://codecov.io/gh/pandas-dev/pandas/pull/20613?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/4afc7563895830d224ac949f571edef2f069c314?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/20613/graphs/tree.svg?token=eZ4WkYLtcO&width=650&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/20613?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #20613      +/-   ##
==========================================
+ Coverage   91.78%   91.78%   +<.01%     
==========================================
  Files         153      153              
  Lines       49324    49348      +24     
==========================================
+ Hits        45272    45296      +24     
  Misses       4052     4052
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.18% <100%> (ø)` | :arrow_up: |
| #single | `41.94% <71.42%> (+0.02%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/20613?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/groupby/groupby.py](https://codecov.io/gh/pandas-dev/pandas/pull/20613/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS9ncm91cGJ5LnB5) | `92.55% <ø> (ø)` | :arrow_up: |
| [pandas/core/panel.py](https://codecov.io/gh/pandas-dev/pandas/pull/20613/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcGFuZWwucHk=) | `97.29% <ø> (ø)` | :arrow_up: |
| [pandas/core/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/20613/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYmFzZS5weQ==) | `96.83% <100%> (ø)` | :arrow_up: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/20613/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.22% <100%> (ø)` | :arrow_up: |
| [pandas/core/reshape/pivot.py](https://codecov.io/gh/pandas-dev/pandas/pull/20613/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9waXZvdC5weQ==) | `96.97% <100%> (ø)` | :arrow_up: |
| [pandas/core/reshape/concat.py](https://codecov.io/gh/pandas-dev/pandas/pull/20613/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9jb25jYXQucHk=) | `97.59% <100%> (ø)` | :arrow_up: |
| [pandas/core/indexes/api.py](https://codecov.io/gh/pandas-dev/pandas/pull/20613/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9hcGkucHk=) | `98.92% <100%> (+0.14%)` | :arrow_up: |
| [pandas/core/indexes/timedeltas.py](https://codecov.io/gh/pandas-dev/pandas/pull/20613/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy90aW1lZGVsdGFzLnB5) | `91.15% <0%> (-0.07%)` | :arrow_down: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/20613/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `95.73% <0%> (-0.04%)` | :arrow_down: |
| [pandas/io/pytables.py](https://codecov.io/gh/pandas-dev/pandas/pull/20613/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3B5dGFibGVzLnB5) | `92.41% <0%> (ø)` | :arrow_up: |
| ... and [3 more](https://codecov.io/gh/pandas-dev/pandas/pull/20613/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/20613?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/20613?src=pr&el=footer). Last update [4afc756...5e1b024](https://codecov.io/gh/pandas-dev/pandas/pull/20613?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
 Thanks for working on this!

* We also need to check how this works in combination with other keywords (eg `join`)
* Do we want to ignore the keyword if all columns match? (what you currently do?)  I would also find that surprising that the keyword then does not work. And given that `sort=False` is the default, I think we can let `sort=True` sort always? @jreback 

I've make the requested changes

@jorisvandenbossche 

* Yeah it does look like `concat` is used a lot internally. I'll have a look at each location its called.
* That sounds reasonable > Yeah it does look like concat is used a lot internally. I'll have a look at each location its called.

Ah, yes, that might be needed as well. But what I meant was other keywords of concat itself. Eg does the sort keyword work for both `join='inner'` and `join='outer'` ?  @brycepg can you update based on Jeff's comments? Doing a release candidate soon (tomorrow or Wednesday hopefully), and it'd be nice to have this in. Sure I’ll try to do it tonight

On Mon, Apr 23, 2018 at 1:15 PM, Tom Augspurger <notifications@github.com> wrote:

> [@brycepg](https://github.com/brycepg) can you update based on Jeff's comments? Doing a release candidate soon (tomorrow or Wednesday hopefully), and it'd be nice to have this in.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, [view it on GitHub](https://github.com/pandas-dev/pandas/pull/20613#issuecomment-383690194), or [mute the thread](https://github.com/notifications/unsubscribe-auth/AAa9yC_mpbFjUbWvFWVaeM4sWVQ3pxffks5trig-gaJpZM4THzks).
>
> {"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/pandas-dev/pandas","title":"pandas-dev/pandas","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/pandas-dev/pandas"}},"updates":{"snippets":[{"icon":"PERSON","message":"@TomAugspurger in #20613: @brycepg can you update based on Jeff's comments? Doing a release candidate soon (tomorrow or Wednesday hopefully), and it'd be nice to have this in."}],"action":{"name":"View Pull Request","url":"https://github.com/pandas-dev/pandas/pull/20613#issuecomment-383690194"}}} @brycepg I pushed some changes to your branch.

1. I updated the API to keep this backwards compat, but you'll get a warning.
2. I updated the docs to explain that

Will you have time today to get go through and update the tests to pass `sort=True` where necessary? The build log will have a bunch of warnings when it finishes.

And I didn't address most of @jreback's comments yet.

I'd like to do a release candidate tomorrow, so if you can't get to it today let me know and I'll push more fixes here. Picking this up. Would be good to have for 0.23. Can someone proofread the warning text? Specifically, does the term "non-concatenation axis" make sense? I could also call it the "non-expanding axis", or something else entirely. > Ah, yes, that might be needed as well. But what I meant was other keywords of concat itself. Eg does the sort keyword work for both join='inner' and join='outer' ?

We'll need to address this.

```python
In [2]: df1 = pd.DataFrame({"a": [1, 2], "b": [1, 2], "c": [1, 2]}, columns=['b', 'a', 'c'])

In [3]: df2 = pd.DataFrame({"a": [1, 2], 'c': [3, 4]}, index=[3, 4])

In [4]: pd.concat([df1, df2], join='inner')
Out[4]:
   a  c
0  1  1
1  2  2
3  1  3
4  2  4

In [5]: pd.concat([df1, df2], join='inner', sort=False)
Out[5]:
   a  c
0  1  1
1  2  2
3  1  3
4  2  4
```

I assume we want the same behavior as for `join='outer'`. Sorry, that was a bad example. join='inner' already preserves the column order of the intersection

```python

In [4]:     df1 = pd.DataFrame({"a": [1, 2], "b": [1, 2], "c": [1, 2]},
   ...:                        columns=['b', 'a', 'c'])
   ...:     df2 = pd.DataFrame({"a": [1, 2], 'b': [3, 4]}, index=[3, 4])
   ...:     result = pd.concat([df1, df2], join='inner')
   ...:
   ...:     result
   ...:
   ...:
Out[4]:
   b  a
0  1  1
1  2  2
3  3  1
4  4  2
```

I'll document that `sort` has no effect for `join='inner'`. > Do we want to ignore the keyword if all columns match?

This depends a bit on the eventual fate of the `sort` keyword. While we may not ever remove it, since we don't want to introduce a keyword and deprecate it shortly after, we can at least discourage its use once the default has been changed. I could imagine the docstring for `sort` eventually just being "for backwards compatibility. Has no effect."

If people want to sort, they should sort that axis and then reindex before or after the concat. > This depends a bit on the eventual fate of the sort keyword.

What's the problem with keeping the keyword, and let it mean to always sort? (sorry if this makes no sense, it's a while ago I looked at this) No problem, just not useful I think.

Though passing sort=True and not having it sort, for already aligned, would be strange. I’ll push an update for that later today.  Found one more issue in crosstab. Fixing now. K, I'll try to go through all the internal calls to concat and set explicitly.

One thing that's made this difficult is that we don't really have the old option of *sort only if not aligned* anymore. So just setting `sort=True` or `sort=False` won't necessarily reproduce the old output. Maybe all of these will be OK though. I'm going to push this to 0.23.1. I'm not confident enough that we have complete test coverage for all the cases of concat used internally.

I can pick it up after PyCon, or you're welcome to work on it whenever @brycepg. If we don't include it in 0.23, I think it has to wait for 0.24. We shouldn't introduce new deprecation warnings in a bug fix release.

However, I don't fully understand the problem to not merge it now. There are still some internal use cases that you need to check? (whether `sort=True/False` needs to be added?) 
But as far as I see, there are no failing tests? Are there still warnings being raised? 
I think in many cases in internal code, it's concatting aligned results (eg in groupby), and then there should be no change or warning.
 Yeah. The concern is about internal calls to concat.

For example, `Panel({b: df, a: df})` was just broken on this branch for a
while with certain conditions. None of the constructor tests caught that,
it was just in a pytables test that it was caught. Though perhaps panels
isn't the best example, not sure how coverage is on that in general.

On Tue, May 1, 2018 at 2:24 PM, Joris Van den Bossche <
notifications@github.com> wrote:

> If we don't include it in 0.23, I think it has to wait for 0.24. We
> shouldn't introduce new deprecation warnings in a bug fix release.
>
> However, I don't fully understand the problem to not merge it now. There
> are still some internal use cases that you need to check? (whether
> sort=True/False needs to be added?)
> But as far as I see, there are no failing tests? Are there still warnings
> being raised?
> I think in many cases in internal code, it's concatting aligned results
> (eg in groupby), and then there should be no change or warning.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/20613#issuecomment-385763928>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIuROG_dJhxtzOuPjLcHphSagFb0Pks5tuLZSgaJpZM4THzks>
> .
>
 In what way was it broken? (there should for now mainly be a warning and new keyword, but not really change in default behaviour?)

>  I'm not confident enough that we have complete test coverage for all the cases of concat used internally.

I can't really say anything founded, as I didn't look enough into detail to what tests have broken throughout implementing this and what you needed to add, but since our tests are now passing, my gut feeling would say: let's use the rc period to get more real-world testing on that .. ;)
 > In what way was it broken? (there should for now mainly be a warning and new keyword, but not really change in default behaviour?)

I don't recall the exact circumstances, but a MultiIndex was (or wasn't?) being sorting.

If you're OK with merging as is, then I can commit to adding more tests (and fixing bugs) between now and Friday. I just don't think it should hold up the release.

 As I said, difficult to say myself, but I am OK with relying on your judgement here. 
(and I certainly approve the gist of the PR) Alright, let's do it.

https://github.com/pandas-dev/pandas/issues/20909 for the followup.