Repository: modin-project/modin
Pull request: FIX-#1497: Don't sort in concat() when sort=False
<!--
Thank you for your contribution! 
Please review the contributing docs: https://modin.readthedocs.io/en/latest/contributing.html
if you have questions about contributing.
-->

## What do these changes do?

<!-- Please give a short brief about these changes. -->

- [x] commit message follows format outlined [here](https://modin.readthedocs.io/en/latest/contributing.html)
- [x] passes `flake8 modin`
- [x] passes `black --check modin`
- [x] signed commit with `git commit -s` <!-- you can amend your commit with a signature via `git commit -amend -s` -->
- [x] Resolves #1497 
- [x] tests added and passing


Problem statement:
concat ignores sort argument
### System information
- **OS Platform and Distribution (e.g., Linux Ubuntu 16.04)**: Ubuntu 19.04
- **Modin version** (`modin.__version__`): 0.7.3+52.g880545b
- **Python version**: Python 3.7.3
- **Code we can use to reproduce**:

<!--
You can obtain the Modin version with

python -c "import modin; print(modin.__version__)"
-->

### Describe the problem
<!-- Describe the problem clearly here. -->
sort argument in modin.pandas.concat calls doesn't affect the result. It works like sort is always True.

### Source code / logs
<!-- Include any logs or source code that would be helpful to diagnose the problem. If including tracebacks, please include the full traceback. Large logs and files should be attached. Try to provide a reproducible test case that is the bare minimum necessary to generate the problem. -->
import modin.pandas as pd

a = pd.DataFrame({'c' : [3],
                  'd' : [4]},
                 columns=list('dc'))
b = pd.DataFrame({'a' : [1],
                  'b' : [2]},
                 columns=list('ba'))

print(pd.concat([a, b], sort=True))
print(pd.concat([a, b], sort=False))

Hints from issue discussion:
I will see if I can fix this.

Commit message: FIX-#1497: Assert sort=False is respected.

Signed-off-by: Itamar Turner-Trauring <itamar@itamarst.org>

Patch to review:
diff --git a/modin/engines/base/frame/data.py b/modin/engines/base/frame/data.py
index d59fbba5dc8..a1583fa8a19 100644
--- a/modin/engines/base/frame/data.py
+++ b/modin/engines/base/frame/data.py
@@ -896,30 +896,37 @@ def _join_index_objects(self, axis, other_index, how, sort):
         """
         Joins a pair of index objects (columns or rows) by a given strategy.
 
+        Unlike Index.join() in Pandas, if axis is 1, the sort is
+        False, and how is "outer", the result will _not_ be sorted.
+
         Parameters
         ----------
-            axis : 0 or 1
-                The axis index object to join (0 - rows, 1 - columns).
-            other_index : Index
-                The other_index to join on.
-            how : {'left', 'right', 'inner', 'outer'}
-                The type of join to join to make.
-            sort : boolean
-                Whether or not to sort the joined index
+
+        axis : 0 or 1 The axis index object to join (0 - rows, 1 - columns).
+        other_index : Index The other_index to join on.  how : {'left',
+        'right', 'inner', 'outer'} The type of join to join to make.  sort :
+        boolean Whether or not to sort the joined index
 
         Returns
         -------
-        Index
-            Joined indices.
+
+        Index Joined indices.
         """
+
+        def merge_index(obj1, obj2):
+            if axis == 1 and how == "outer" and not sort:
+                return obj1.union(obj2, sort=False)
+            else:
+                return obj1.join(obj2, how=how, sort=sort)
+
         if isinstance(other_index, list):
             joined_obj = self.columns if axis else self.index
             # TODO: revisit for performance
             for obj in other_index:
-                joined_obj = joined_obj.join(obj, how=how, sort=sort)
+                joined_obj = merge_index(joined_obj, obj)
             return joined_obj
         if axis:
-            return self.columns.join(other_index, how=how, sort=sort)
+            return merge_index(self.columns, other_index)
         else:
             return self.index.join(other_index, how=how, sort=sort)
 
diff --git a/modin/pandas/dataframe.py b/modin/pandas/dataframe.py
index 62957565fc6..97709de0e31 100644
--- a/modin/pandas/dataframe.py
+++ b/modin/pandas/dataframe.py
@@ -530,6 +530,11 @@ def append(self, other, ignore_index=False, verify_integrity=False, sort=False):
         Returns:
             A new DataFrame containing the concatenated values.
         """
+        if sort is False:
+            warnings.warn(
+                "Due to https://github.com/pandas-dev/pandas/issues/35092, "
+                "Pandas ignores sort=False; Modin correctly does not sort."
+            )
         if isinstance(other, (Series, dict)):
             if isinstance(other, dict):
                 other = Series(other)
diff --git a/modin/pandas/test/test_concat.py b/modin/pandas/test/test_concat.py
index 9d975b0bd29..f1caf30fc81 100644
--- a/modin/pandas/test/test_concat.py
+++ b/modin/pandas/test/test_concat.py
@@ -201,3 +201,18 @@ def test_concat_dictionary(axis):
         pd.concat({"A": modin_df, "B": modin_df2}, axis=axis),
         pandas.concat({"A": pandas_df, "B": pandas_df2}, axis=axis),
     )
+
+
+@pytest.mark.parametrize("sort", [False, True])
+@pytest.mark.parametrize("join", ["inner", "outer"])
+@pytest.mark.parametrize("axis", [0, 1])
+def test_sort_order(sort, join, axis):
+    pandas_df = pandas.DataFrame({"c": [3], "d": [4]}, columns=["d", "c"])
+    pandas_df2 = pandas.DataFrame({"a": [1], "b": [2]}, columns=["b", "a"])
+    modin_df, modin_df2 = from_pandas(pandas_df), from_pandas(pandas_df2)
+    pandas_concat = pandas.concat([pandas_df, pandas_df2], join=join, sort=sort)
+    modin_concat = pd.concat([modin_df, modin_df2], join=join, sort=sort)
+    df_equals(
+        pandas_concat, modin_concat,
+    )
+    assert list(pandas_concat.columns) == list(modin_concat.columns)
diff --git a/modin/pandas/test/test_dataframe.py b/modin/pandas/test/test_dataframe.py
index a92b5d54a97..cd309cec3d8 100644
--- a/modin/pandas/test/test_dataframe.py
+++ b/modin/pandas/test/test_dataframe.py
@@ -745,6 +745,14 @@ def test_append(self, data):
                 modin_df.append(list(modin_df.iloc[-1]))
         else:
             modin_result = modin_df.append(list(modin_df.iloc[-1]))
+            # Pandas has bug where sort=False is ignored
+            # (https://github.com/pandas-dev/pandas/issues/35092), but Modin
+            # now does the right thing, so for now manually sort to workaround
+            # this. Once the Pandas bug is fixed and Modin upgrades to that
+            # Pandas release, this sort will cause the test to fail, and the
+            # next two lines should be deleted.
+            assert list(modin_result.columns) == list(modin_df.columns) + [0]
+            modin_result = modin_result[[0] + sorted(modin_df.columns)]
             df_equals(modin_result, pandas_result)
 
         verify_integrity_values = [True, False]


Code review comments:
diff_hunk: @@ -896,30 +896,37 @@ def _join_index_objects(self, axis, other_index, how, sort):
         """
         Joins a pair of index objects (columns or rows) by a given strategy.
 
+        Unlike Index.join() in Pandas, if axis is 1, the sort is
+        False, and how is "outer", the result will _not_ be sorted.
+
         Parameters
         ----------
-            axis : 0 or 1
-                The axis index object to join (0 - rows, 1 - columns).
-            other_index : Index
-                The other_index to join on.
-            how : {'left', 'right', 'inner', 'outer'}
-                The type of join to join to make.
-            sort : boolean
-                Whether or not to sort the joined index
+
+        axis : 0 or 1 The axis index object to join (0 - rows, 1 - columns).
path: modin/engines/base/frame/data.py
text: @user1:
I think this was accidentally changed, please revert these documentation formatting changes.

@author:
Done.