BrandonFors/pandas-github-issues huggingface dataset:
API/DEPR: Deprecate order kwarg in factorize
State: closed

Closes https://github.com/pydata/pandas/issues/6926

`order` wasn't being used at all.

I couldn't use the deprecate_kwarg decorator since the kwarg is being removed, not changed.


Labels:
name: API Design
name: Deprecate
description: Functionality to remove in pandas

Comments:
make sure to add to #6581
API/DEPR: Match Panel.shift()'s signature to generic
State: closed

Closes https://github.com/pydata/pandas/issues/6910

Also switches the shift implementation back to using keyword arguments.


Labels:
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: API Design
name: Deprecate
description: Functionality to remove in pandas

Comments:
Added to #6581

looks ok
merge when green (and mark labels on the pr and issue as appropriate)
API: remove the copy kw from .xs to prevent an expectation of a view (which may not be possible)
State: closed

closes #6894

Deprecate passing keyword `copy` to:
- `DataFrame/Series/Panel/.xs`
- `Panel.minor_xs`
- `Panel.major_xs`

`.xs` returns a view (as does `.loc`) **IF POSSIBLE**

MultiIndexing with Slicers eliminates the need for this (and `.xs` for the too, but for
some cases its 'simpler').


Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: API Design

Comments:
Should we go first through a deprecation cycle? So to not let fail your code when you provided the `copy` keyword, but only warning it is ignored and will be removed?

good point..will add on the user facing code 

@jorisvandenbossche updated.....so this still allows one to do

```
df.xs('foo')[:] = 10
```

but now is undocumented (was in a doc string before)
(and will give a `SettingWithCopyWarning` if its actually a copy, rather than a view).

But that is not supported / dangerous anyhow

MultiIndex Slicers is the correct method

```
df.loc['foo'] = 10
```

so nothing lost

maybe should add to the doc string that `.xs` is only for getting?

@jorisvandenbossche how do I do a `See also` and have it refer to `loc`? in the docstring (so you can click it)

@jorisvandenbossche 

can I use a `:ref:` directly in the See Also?

e.g.

```
For more information on ``.at``, ``.iat``, ``.ix``, ``.loc``, and
``.iloc``,  see the :ref:`indexing documentation <indexing>`.
```

like this:

```
See also
--------
DataFrame.loc : some explanation why this would be related
```

only the docstring of `loc` is not that informative ... (http://pandas.pydata.org/pandas-docs/dev/generated/pandas.DataFrame.loc.html#pandas.DataFrame.loc) maybe should open an issue about that

just did #6920

Aha :-)

Yes, you can use all sphinx referencing in the docstrings as in the normal docs (only if you see the docstring as plain text in eg the terminal, then it makes less sense, but for the api pages on the online docs this is very useful)

gr8..makes sense

Maybe explicitly add in the description of `xs` is only for getting? I found that a good idea. 
And the `See also` has to be a `Notes` now ('see also' is really only for python/pandas objects that sphinx can make a link to, and not for a general sentence)
DOC: Add recipe for shifting groups of values based on the index
State: closed


Labels:
name: Docs

Comments:
So edited.

@unutbu thanks!

It think in 0.14 enabled `.shift(1)` to work directly, rathern than `.apply(lambda grp: grp.shift(1))`

i'll fixup

Ah. That works as far back as 0.8.0 as well.

Just for fun I wanted to use `git bisect` to find the first commit where `groupby(...).shift(1)` works. I can do it manually, but this requires that I reinstall pandas to test each commit:

```
/bin/rm -rf ~/src/pandas/{build,dist} && cdsitepackages && /bin/rm -rf pandas*
cd ~/src/pandas && python setup.py build_ext --inplace && python setup.py install 
```

I suppose I could write a unittest script with a setUp method which performs the installation, and then run `git bisect run ...`. But is there an easier, automated way?

ahh...that's the automatic function call generation, kind of a syntactic sugar

I don't any easier way to run git bisect....only used it a couple of times.

I think bisect will do the install for you (e.g. it checkouts a particular commit), runs setup, then runs the code....
to_csv `headers` kwarg now works regardless of `index` kwarg
State: closed

Addresses issue https://github.com/pydata/pandas/issues/6186


Labels:

Comments:
pls add a release note referring to the issue

Sorry I'm new to the PANDAS style guide: There's a note in the unit test---should I also put a note in `format.py`?

no
need a note in doc/source/release.rst

Just added it---look good?

this looks good, and DRYer to boot. Awesome!
BUG: GroupBy.get_group doesnt work with TimeGrouper
State: closed

`get_group` raises `AttributeError` when the group is created by `TimeGrouper`.

```
>>> df = pd.DataFrame({'Branch' : 'A A A A A A A B'.split(),
                   'Buyer': 'Carl Mark Carl Carl Joe Joe Joe Carl'.split(),
                   'Quantity': [1,3,5,1,8,1,9,3],
                   'Date' : [
                    datetime(2013,1,1,13,0), datetime(2013,1,1,13,5),
                    datetime(2013,10,1,20,0), datetime(2013,10,2,10,0),
                    datetime(2013,10,1,20,0), datetime(2013,10,2,10,0),
                    datetime(2013,12,2,12,0), datetime(2013,12,2,14,0),]})

>>> grouped = df.groupby(pd.Grouper(freq='1M',key='Date'))
>>> grouped.get_group(pd.Timestamp('2013-12-31'))
AttributeError: 'DataFrameGroupBy' object has no attribute 'indices'
```


Labels:
name: Bug
name: Groupby

Comments:
Thanks, this is definitely a bug and tests look good!

I reckon there's a more efficient way to get indices though.

@sinhrks as a side note, pls make sure that your examples are easily copy-pasted (e.g. put the `from datetime import datetime`), though IMHO, pls just `import datetime`

put this release note next to or with the one for #5267

@sinhrks ping when you update

ok...this is fine, if you think of someway to reuse the current groupby functions for that `indicies` pls do a pr in the future

thanks!
ENH: pivot_table can now accept Grouper
State: closed

`pivot_table` can accept `Grouper` by `index` and `columns` kw.

```
>>> df = pd.DataFrame({
    'Branch' : 'A A A A A A A B'.split(),
    'Buyer': 'Carl Mark Carl Carl Joe Joe Joe Carl'.split(),
    'Quantity': [1,3,5,1,8,1,9,3],
    'Date' : [datetime(2013,11,1,13,0), datetime(2013,9,1,13,5),
              datetime(2013,10,1,20,0), datetime(2013,10,2,10,0),
              datetime(2013,11,1,20,0), datetime(2013,10,2,10,0),
              datetime(2013,10,2,12,0), datetime(2013,12,5,14,0)],
    'PayDay' : [datetime(2013,10,4,0,0), datetime(2013,10,15,13,5),
                datetime(2013,9,5,20,0), datetime(2013,11,2,10,0),
                datetime(2013,10,7,20,0), datetime(2013,9,5,10,0),
                datetime(2013,12,30,12,0), datetime(2013,11,20,14,0),]})

>>> pd.pivot_table(df, index=pd.Grouper(freq='M', key='Date'),
                     columns=pd.Grouper(freq='M', key='PayDay'),
                     values='Quantity', aggfunc=np.sum)
PayDay      2013-09-30  2013-10-31  2013-11-30  2013-12-31
Date                                                      
2013-09-30         NaN           3         NaN         NaN
2013-10-31           6         NaN           1           9
2013-11-30         NaN           9         NaN         NaN
2013-12-31         NaN         NaN           3         NaN
```


Labels:
name: Groupby
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: API Design

Comments:
Looks usefull at first glance!
- Can you in the future first open an issue to propose the functionality? (unless there is already one?) In this case I personally think it is useful functionality, but if we would decide this is out of scope for pandas (or some other reason to not include it), it would be a bit pity for your efforts in the PR.
- Can you also add something to the docs (somewhere in reshaping.rst)
- I suppose this is only usefull for datetime-like columns? Maybe explain this better then?

@sinhrks ping when you update

@jreback @jorisvandenbossche Updated the points and rebased. Pls check?

@jreback docs are OK now for me. But I don't really have a judgement on the implemenation. OK for you?

thanks @sinhrks
ENH: Implement Panel pct_change
State: closed

Closes https://github.com/pydata/pandas/issues/6904

There's just a bit of extra index handling that needs to be done before moving on to `generic.pct_change()`. I had to adjust that to use the `.div` and `.sub` ops instead of `/` and `-` to work with panels.

I wasn't sure why axis wasn't included as an actual names keyword arg. `generic` just looks for it in **kwargs. I did the same in panel.

A related issue was the `Panel.shift()` has a different argument signature than `generic.shift()`. I can make those consistent and put in a deprecation warning in this issue or in a new one.


Labels:
name: Bug
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: API Design
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations

Comments:
I added a vbench, but maybe it isn't worth it since really all that's happing is a panel.shift() and a panel.div(). Should I take it out?

it's fine

instead of specifying the items axis use info_axis for ndim > 2

maybe this should also be explict for ndim > 2
iow you must specify an axis ?
not sure if we do that in other functions

I changed the default axis to major to be more consistent with `panel.shift()`. Wasn't sure what you meant about making it explicit. Do you mean adding the axis as a real kwarg?

axis should default to stat_axis (which is 1 for a panel)

I finally understand what you meant about the axes (I've never really understood what `info_axis` and `stat_axis` were before now). This let me move everything to generic. Should be good now. Haven't tested with Panel4D and up, but in principle there's no reason it shouldn't work.

gr8 - generic is good! makes maint easier

u can simply put in the Panel4D notimementrd list (as this is prob not intuitive), look in panelnd for a list of functions that it disables

Actually this will already raise NotImplemented since `shift` is on the NotImplemented list, and that's used in pct_change.

ok gr8
DOC: note about query strings in HDFStore selection
State: closed

closes #6901


Labels:
name: Docs
name: IO HDF5
description: read_hdf, HDFStore

Comments:
See #6901. That was the reported error, so I wanted to make that example explicit. It will fail with a syntax error if you interpolate the string into a query

I'll point out the single and double quote

maybe show that u can pass a string if u surround it with quotes 

done

turning out to be more controversial than i would've expected

the locals interpolation is a bit magical (but useful)

I always use direct quotes -

speaking of that, i must've overlooked this when refactoring local vars API but shouldn't pytables follow that convention?

it should follow the same convention
problem?

i meant with the `@` syntax. ... that doesn't seem to work

in the example from #6901 

hmm
ok so is this more like eval or query?
it doesn't have context so maybe should be more like eval in that locals need @ ?

i'll make a new issue, merge this doc PR?

yep
API: Stop modifying SQL column and names, and warn when pertinent.
State: closed

Closes #6796

If any columns names written `to_sql` via a legacy connection _would_ have been modified by previous versions of pandas, a warning is issued, both on table creation and row insertion. But no names are modified.

There is a stray warning message in nosetests. Otherwise good, I think. Does this reflect our consensus?


Labels:
name: IO SQL
description: to_sql, read_sql, read_sql_query

Comments:
Looking good, I think this does reflect consensus!

Maybe also add a test to check that a column name with a space is written correctly?

And what warning messages do you get in nose?

Edited in response to @jorisvandenbossche's review _and_ added a test for a bug in the original PR: DataFrame column names must be converted to strings.

Ah, good, the integer column name error was still a to-fix on the #6292 list. Checked it as fixed!

Looking good for the rest. Ready to merge for you?

nosetests still spews a straw warning. Know a trick for finding out which test is to blame? I am pretty sure `tm.assert_produces_warning` suppresses the warning, so this is leaking out from some other test.

*straw -> stray

which warning do you get?

If you just execute the file (tests_sql.py, there is a main that calls nose), then I see the progress of the tests in my terminal, and you can see for which test it is generated.

BTW, there is still a warning that you should not care about for the moment (about not being a SQLAlchemy engine, at the moment you get that always when using the legacy, so just the first test using a DBAPI connection is triggering this).

Oh OK. I thought that warning was due to me. Then this is ready to merge.

No it was not you, but I think we should do something about the warning (but that is a discussion for legacy-status issue).

Merged, thanks!
SQL: add multi-index support to legacy mode
State: closed

Closes #6881.

@danielballan Added multi-index support to legacy mode, wasn't actually not that difficult (all tests seems to pass). And at once also `index_label` kwarg is supported in legacy mode.

This just leaves the issue that this is actually an api change, so could possibly break code.


Labels:

Comments:
That _was_ quick. This seems like the best outcome from an assortment of confusing compromises, any of which could mean trouble for legacy code. Looks good to me.

I will proceed with #6796 once this is merged.

@jreback @hayd What do you think of the API change (before: no index was written, now `index` kwarg to write index which is set to `True` by default)?

Actually, we could also set `index` to `False` when using `write_frame`, and leave it as `True` for `to_sql`. `to_sql` is a new function, so for that the backwards incompatibility is not a problem.

(at this moment, `write_frame` is just passed in its entirety to `to_sql`)

@jreback @hayd OK with this? I added a commit that changes the default `index` to False for `write_frame` (behaviour of 0.13), for `to_sql` (new function) it is then `index=True`. As this is the most backwards compatible I suppose no objections?

looks ok

Ahum, I was actually completely wrong here... `to_sql` did already exist of course, it was added in 0.12. It was just not in the `sql` module but only as a method of DataFrame where it called directly `write_frame`, and it was not mentioned in the docs.
So my reasoning was not correct, and for `to_sql` this remains a backwards incompatible api change.

Bummer. But I still think this is the lesser evil. Dropping the index was always a weird behavior, an indication that the SQL capability wasn't mature and should be used cautiously.
CLN: simplify series plotting
State: closed

Modified these 2 points:
- Simplify `LinePlot._make_ts_plot` to have single flow.
- Simplify `plot_frame` and `plot_series` 's `kind` detection. (Also, setting hexbin default is not necessary because it is done in `HexBinPlot.__init__`)


Labels:
name: Visualization
description: plotting

Comments:
looks fine and +1 for simplifying the code a little, going to leave it open for a bit for comments (if any) then merge

thanks!
API: update SQL functional api (GH6300)
State: closed

First draft for the functional api part of #6300.


Labels:
name: IO SQL
description: to_sql, read_sql, read_sql_query

Comments:
OK, I think this is fine now. I only changed the toplevel api (so `read_sql` -> `read_sql_query`, `read_table` -> `read_sql_table` and a new `read_sql` as wrapper around both), and not yet the methods of the PandasSQLAlchemy objects (as we did not yet discuss that).

Somebody care to do a quick review? @hayd @jreback @danielballan @mangecoeur

Other than those minors docstring suggestions, looks great to me.

Looks good (sorry, thought I commented earlier!)

Thanks for the comments! Adapted the summary lines of the docstrings a bit. 
Merging now.
Custom Business Month
State: closed

This extends the `offsets.py` module with a `CBMonthEnd` based on `CustomBusinessDay`. Tests are found in `test_offsets.py`. 

A few small points still open: 
- Implement CBMonthBegin
- Improve speed. Main speed drag is `DateOffset.onOffset`. 

Also some of the tests are not passing, but the errors seem unrelated. 
e.g.

```
======================================================================
ERROR: test_pickle (pandas.tseries.tests.test_timeseries.TestTimeSeries)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/benjamin/workspace/pandas/pandas/tseries/tests/test_timeseries.py", line 1827, in test_pickle
    p = pick.loads(pick.dumps(NaT))
  File "/usr/lib/python2.7/pickle.py", line 1382, in loads
    return Unpickler(file).load()
  File "/usr/lib/python2.7/pickle.py", line 858, in load
    dispatch[key](self)
  File "/usr/lib/python2.7/pickle.py", line 1133, in load_reduce
    value = func(*args)
TypeError: __new__() takes exactly one argument (2 given)
```


Labels:
name: Enhancement
name: Frequency
description: DateOffsets

Comments:
did u rebuild extensions? this was just added in cython

python setup.py build_ext --inplace

Ok that was part of it. Apparently Travis is on numpy 1.6 so the CBMonth (requires numpy 1.7) tests are failing. 

their are multiple versions of numpy tested with Travis 
look at the very end of the build u can see the print_versions

Ah good hint, found the problem, will send an update later tonight. 

I added `CBMonthBegin` and squashed the changes. Anything else to do? 

pls rebase (u need to incorporate #6873 the apply_nat)

How should I reference the enhancement in `v0.14.0.txt`? :issue:`6866` ? 

yep (when no associated issue the pr number works too)

Rebased, added a small section to the docs. Anything else? 

can u do the Apply_nat decorator like other apply? it's just more consistent that way

@jorisvandenbossche @TomAugspurger 

comments?

@bjonen can you rebase?

done

thank you sir!

FYI, had to do this little patch https://github.com/pydata/pandas/commit/a189d5d0775e46cb8fc2a68955c85e499ed04e6c

was failing on windows 2.6 for some reason (and not sure why didn't fail on linux)

Ok, thanks!
BUG: _tidy_repr should not be called when max_rows is None
State: closed

This issue was raised in http://stackoverflow.com/q/22824104/190597:

```
import pandas as pd
pd.options.display.max_rows = None
result = pd.Series(range(1001))
print(result)
```

raises `TypeError: unsupported operand type(s) for -: 'NoneType' and 'int'`.

The problem occurs in series.py  (line 832) when `max_rows` is `None`:

```
    if len(self.index) > (max_rows or 1000):
        result = self._tidy_repr(min(30, max_rows - 4))
```

Since the doc string for `get_options` says

```
display.max_rows: [default: 60] [currently: 60]
        ...
        'None' value means unlimited.
```

I think `_tidy_repr` should not be called when `max_rows` is None.

This PR seems simple enough but my main concern is that  this PR stomps on GH1467 which explicitly changed `> max_rows` to `> (max_rows or 1000)` and I don't know what the purpose of this was.


Labels:
name: Bug
name: Output-Formatting
description: __repr__ of pandas objects, to_string

Comments:
can you add a test, something like:

```
with pd.option_context('max_rows',None):
     str(Series(range(1001)))
```

which would raise w/o this patch but pass with it

test added.

can u post a before / after (maybe with defaults and maybe max_rows a bigger number)

to see what this is actually doing

I'm sorry, I'm not following you. If `max_rows` is set to any `int`, e.g. 0, 60, 2000, then there is no Exception. The exception is raised only when `max_rows` is `None`.

so in reality, `max_rows` of 0 or None is the same effectively as `max_rows` of 1000 (when this patch is applied)

Using master, `max_rows == 0` would return a summarized repr only if `len(series.index) > 1000`.

After this patch is applied, `max_rows == 0` would result in a summarized `repr` if `len(series.index) > 0`.

I'm not sure what `max_rows = 0` is supposed to do. I could preserve the old behavior with

```
    if max_rows is not None and len(self.index) > (max_rows or 1000):
```

however.

i think `max_rows=0` and `max_rows=None` are equivalent, so that is fine

can you add release note (ref this PR), in Bug Fixes I think

@TomAugspurger @jorisvandenbossche any objections?

If `max_rows = 0` and `max_rows = None` should behave the same, then perhaps I should change it to  `if max_rows` instead of `if max_rows is not None`:

```
if max_rows and len(self.index) > max_rows:
    result = self._tidy_repr(min(30, max_rows - 4))
```

this options interplay always confuses me. that seems reasonable as what is the point of max_rows=0 anyhow, yes, I think that should be equivalent to None

Added note to Bug Fixes in `release.rst`, and now using `if max_rows` to handle `max_rows = 0` the same as `max_rows = None`.

+1 to treating `max_rows=0` the same as `max_rows=None`. The alternative of not printing any rows would be more surprising than printing all the rows. 

@unutbu thanks as always!
API: add inplace keyword to Series.order/sort to make them inverses (GH6859)
State: closed

closes #6859


Labels:
name: API Design

Comments:
now to be even more confusing

`DataFrame.sort(inplace=False)` is the default (and returns a new Frame)

`Series.sort(inplace=True)`
`Seires.order(inplace=False)`

To fix this is REALLY breaking the API

maybe add `DataFrame.order(inplace=False)`

and FutureWarn that `DataFrame.sort(inplace=False)` should be using order?
DEPR: Indexers will warn FutureWarning when used with a scalar indexer this is floating-point (GH4892)
State: closed

closes #4892

numpy is doing this i 1.9 IIRC, and Scipy already did it. in 0.13.3

 Indexers will warn `FutureWarning` when used with a scalar indexer and a non-floating point Index

```
# non-floating point indexes can only be indexed by integers / labels
In [1]: Series(1,np.arange(5))[3.0]
        pandas/core/index.py:469: FutureWarning: scalar indexers should not be floating point unless a    floating index
Out[1]: 1

In [5]: Series(1,np.arange(5)).iloc[3.0]
        pandas/core/index.py:463: FutureWarning: scalar indexers should not be floating point unless a floating index
Out[5]: 1

# these are Float64Indexes, so integer or floating point is acceptable
In [3]: Series(1,np.arange(5.))[3]
Out[3]: 1

In [4]: Series(1,np.arange(5.))[3.0]
Out[4]: 1
```


Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Deprecate
description: Functionality to remove in pandas

Comments:
I know what you mean, but this error message is a little impenetrable.
Maybe we should say: FutureWarning: scalar indexers for {klass} should be
int and not float.

And fill in klass with the index type.

ok...sure!

I think that ALL warnings should purposely be as confusing as possible!

Haha

```
In [2]: Series(1,np.arange(5)).iloc[3.0]
pandas/core/index.py:463: FutureWarning: scalar indexers for index type Int64Index should be integers and not floating point
```