Concat with axis rows #14416

brandonmburroughs · 2016-10-13T14:37:04Z

closes concat with axis='rows' #14369
tests added / passed
passes git diff upstream/master | flake8 --diff
whatsnew entry

I closed #14389 and opened with one in hopes of the new commits showing up.

Adding tests for concat string axis names Fixing pep8 related spacing

paul-mannino · 2016-10-13T16:16:43Z

Edit: Didn't realize you already knew this/it was just weirdness with the repo

brandonmburroughs · 2016-10-13T18:48:23Z

Thanks for the tip @paul-mannino ! I'll do that next time.

codecov-io · 2016-10-13T18:57:47Z

Current coverage is 85.26% (diff: 100%)

Merging #14416 into master will increase coverage by <.01%

@@             master     #14416   diff @@
==========================================
  Files           140        140          
  Lines         50634      50642     +8   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43173      43181     +8   
  Misses         7461       7461          
  Partials          0          0

Powered by Codecov. Last update 7d40f18...9aa260d

jorisvandenbossche · 2016-10-13T19:23:04Z

pandas/tools/merge.py

@@ -1411,6 +1412,10 @@ def __init__(self, objs, axis=0, join='outer', join_axes=None,
            sample = objs[0]
        self.objs = objs

+        # Check for string axis parameter
+        if isinstance(axis, str):


the check for string is not needed, this is already handled inside get_axis_number, so you can just do axis = get_axis_number(axis)

jorisvandenbossche · 2016-10-13T19:24:40Z

pandas/tools/util.py

+    ----------
+    axis : str
+        Only values 'index', 'columns', or 'rows' will return a value.  Other
+        values will raise a ValueError exception


This is not fully correct, as the function also checks for integers

jreback · 2016-10-13T20:58:07Z

pandas/tools/util.py

@@ -238,3 +238,39 @@ def to_numeric(arg, errors='raise', downcast=None):
        return values[0]
    else:
        return values
+
+


this is not the right place for this at all. remove all of this added code from tools/util (and the tests)

In [2]: pd.DataFrame()._get_axis_number(1) Out[2]: 1 In [3]: pd.DataFrame()._get_axis_number('index') Out[3]: 0 In [4]: pd.DataFrame()._get_axis_number('columns') Out[4]: 1 In [5]: pd.DataFrame()._get_axis_number(1) Out[5]: 1

Thanks for this. Just to make sure I understand everything, this is better than the original solution, because it doesn't depend upon the object passed to concat yet still uses the DataFrame method without having to reproduce it (and add unnecessary duplication)? And there isn't any problem with creating an empty DataFrame instance in order to use the method because we can consistently create it every time?

This is my first PR to Pandas, so I appreciate the help!

no that is all fine.

Okay thanks. I believed I've fixed it now.

brandonmburroughs · 2016-10-15T16:10:48Z

Actually, just using the DataFrame _get_axis_number causes tests with Panels to fail. I've updated it to use the DataFrame method if a Series is passed and to use the object itself (sample) otherwise.

jorisvandenbossche · 2016-10-15T19:31:48Z

Looks good to me

jreback · 2016-10-15T19:59:58Z

thanks!

closes pandas-dev#14369 Author: Brandon M. Burroughs <[email protected]> Closes pandas-dev#14416 from brandonmburroughs/concat_with_axis_rows and squashes the following commits: 9aa260d [Brandon M. Burroughs] Changing axis handling to depend on object passed 49442be [Brandon M. Burroughs] Using dataframe _get_axis_number instance method 64702fb [Brandon M. Burroughs] Updating documentation for concat fdd5260 [Brandon M. Burroughs] Removing duplicate expected dfs 3f08b07 [Brandon M. Burroughs] Adding concat tests for axis 0, 1, and 'index' cf3f998 [Brandon M. Burroughs] Adding ValueError test for concat Series axis 'columns' a6694b9 [Brandon M. Burroughs] Updating documentation 584ebd2 [Brandon M. Burroughs] BUG: Allow concat to take string axis names

closes #14369 Author: Brandon M. Burroughs <[email protected]> Closes #14416 from brandonmburroughs/concat_with_axis_rows and squashes the following commits: 9aa260d [Brandon M. Burroughs] Changing axis handling to depend on object passed 49442be [Brandon M. Burroughs] Using dataframe _get_axis_number instance method 64702fb [Brandon M. Burroughs] Updating documentation for concat fdd5260 [Brandon M. Burroughs] Removing duplicate expected dfs 3f08b07 [Brandon M. Burroughs] Adding concat tests for axis 0, 1, and 'index' cf3f998 [Brandon M. Burroughs] Adding ValueError test for concat Series axis 'columns' a6694b9 [Brandon M. Burroughs] Updating documentation 584ebd2 [Brandon M. Burroughs] BUG: Allow concat to take string axis names (cherry picked from commit fd3be00)

brandonmburroughs added 6 commits October 10, 2016 17:50

BUG: Allow concat to take string axis names

584ebd2

Adding tests for concat string axis names Fixing pep8 related spacing

Updating documentation

a6694b9

Adding ValueError test for concat Series axis 'columns'

cf3f998

Adding concat tests for axis 0, 1, and 'index'

3f08b07

Removing duplicate expected dfs

fdd5260

Updating documentation for concat

64702fb

jorisvandenbossche reviewed Oct 13, 2016

View reviewed changes

brandonmburroughs mentioned this pull request Oct 13, 2016

BUG: Allow concat to take string axis names #14389

Closed

4 tasks

jorisvandenbossche added Bug Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Oct 13, 2016

jorisvandenbossche added this to the 0.19.1 milestone Oct 13, 2016

jreback reviewed Oct 13, 2016

View reviewed changes

Using dataframe _get_axis_number instance method

49442be

brandonmburroughs force-pushed the concat_with_axis_rows branch from 24eb09d to 49442be Compare October 13, 2016 22:09

Changing axis handling to depend on object passed

9aa260d

jreback closed this in fd3be00 Oct 15, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Concat with axis rows #14416

Concat with axis rows #14416

brandonmburroughs commented Oct 13, 2016

paul-mannino commented Oct 13, 2016 •

edited

Loading

brandonmburroughs commented Oct 13, 2016

codecov-io commented Oct 13, 2016 •

edited

Loading

jorisvandenbossche Oct 13, 2016

jorisvandenbossche Oct 13, 2016

jreback Oct 13, 2016

brandonmburroughs Oct 13, 2016

jreback Oct 13, 2016

brandonmburroughs Oct 13, 2016

brandonmburroughs commented Oct 15, 2016

jorisvandenbossche commented Oct 15, 2016

jreback commented Oct 15, 2016

Concat with axis rows #14416

Concat with axis rows #14416

Conversation

brandonmburroughs commented Oct 13, 2016

paul-mannino commented Oct 13, 2016 • edited Loading

brandonmburroughs commented Oct 13, 2016

codecov-io commented Oct 13, 2016 • edited Loading

Current coverage is 85.26% (diff: 100%)

jorisvandenbossche Oct 13, 2016

Choose a reason for hiding this comment

jorisvandenbossche Oct 13, 2016

Choose a reason for hiding this comment

jreback Oct 13, 2016

Choose a reason for hiding this comment

brandonmburroughs Oct 13, 2016

Choose a reason for hiding this comment

jreback Oct 13, 2016

Choose a reason for hiding this comment

brandonmburroughs Oct 13, 2016

Choose a reason for hiding this comment

brandonmburroughs commented Oct 15, 2016

jorisvandenbossche commented Oct 15, 2016

jreback commented Oct 15, 2016

paul-mannino commented Oct 13, 2016 •

edited

Loading

codecov-io commented Oct 13, 2016 •

edited

Loading