BrandonFors/pandas-github-issues huggingface dataset:
CLN Pep8 on format.py
State: closed

While looking at making a separate PR in format.py, I decided to first clean up the file and make it PEP8 compliant. I used 99 maximum-line-length per http://legacy.python.org/dev/peps/pep-0008/#maximum-line-length.

I also noticed that line 965 (in master) has a list comprehension that iterates through a non-instantiated variable (**row**):

``` python
for i in range(nrows):
    if truncate_v and i == (self.fmt.tr_row_num):
        str_sep_row = [ '...' for ele in row ]
        self.write_tr(str_sep_row, indent, self.indent_delta, tags=None, nindex_levels=1)

        row = []
        row.append(index_values[i])
        row.extend(fmt_values[j][i] for j in range(ncols))
```

By the time it has gone through the loop once, **row** is set, but it really should be instantiated ahead of the loop to be safe.

Additionally, I thought it might be useful to add a PEP8 test in test_format, just to make sure that future commits don't break PEP8. 


Labels:
name: Code Style
description: Code style, linting, code_checks

Comments:
@jorisvandenbossche It think we have 80 char limit?

@jreback @jorisvandenbossche let me know if you'd like me to convert this to 80 char. That shouldn't be a big deal. I'm more curious if you think adding a PEP8 test to lock in PEP8 on files as they get converted is a good idea. I think I've got all the places in the docs where dev requirements are discussed. I'll get that in a PR shortly.

in theory once things are pepped the test should enforce it which is a good idea

but not sure if this will get spurious things as pull requests are made 

On the 80 or 100 char limit, I don't think we have it specified somewhere? 

On the PEP8 testing in travis, I think this can be a good idea, but in such a case I think it should be a more general solution. Eg scipy and matplotlib run pep8 compliance tests in travis on the full code base, with some rules and some files ignored. It should also be better it showed what exactly failed (instead of just pass/fail). 
But that needs a seperate issue/mailing thread I think to discuss if we want this (that every PR has to be PEP8 compliant), and what rules we want to ignore for the pandas codebase.

See eg https://github.com/matplotlib/matplotlib/pull/1945 and https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/tests/test_coding_standards.py

@iwschris ok, so let's eliminate the pep8 test for now. And pls create another issue to setup the CI for this type of behavior @jorisvandenbossche mentioned.

No problem pepping things (but do as separate PR's one at a time after rebasing on master). so we can merge quickly.

Will do!

@jreback This should be good to go. I also created an issue for PEP8 Discussion here: https://github.com/pydata/pandas/issues/8317

@iwschris thanks!
BUG: make sure that the multi-index is lex-sorted before passing to _lexsort_indexer (GH8017)
State: closed

BUG: sparse repr of multi-index frame with a FloatIndex as a level was incorrect

closes #8017 


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

Comments:
Not sure if this is the same issue, but I'm getting weird behavior on MultiIndex displays (and possibly in the underlying sortin too) as described in #8017 in my 10/29 update.
ENH groupby: more user-friendly error message (GH7511)
State: closed

better error message for groupy on duplicated columns name. (Followup for branch GH8210 (Bug GH7511)). E.g.

```
ValueError: Grouper for 'A' not 1-dimensional
ValueError: Grouper for '<class '__main__.gr'>' not 1-dimensional
```


Labels:
name: Groupby

Comments:
thanks!
API: add ddof to expanding/rolling_cov()
State: closed

Closes #8279 
closes #8064


Labels:
name: API Design
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations

Comments:
CC @jaimefrio

@seth-p any reason you can think of why you would NOT want to have a ddof? (prob just an oversite when this was initially created)

@jorisvandenbossche 

I can't think of one. Note that `np.std/var()` has a `ddof` argument, though there it defaults to `0`, whereas I think in Pandas it should default to `1` (for backwards compatibility reasons, if nothing else).

As I observed in #8064, `rolling/expanding_std/var()` do already support a `ddof` argument, but it's just not documented. (And it's not obvious to me how to add it to their doc strings.)

I also see no reason.

For adding it to the docs of rolling_std et al, I think you have to make a seperate version of `_rolling_func` (https://github.com/seth-p/pandas/blob/rolling_cov_ddof/pandas/stats/moments.py#L628)  for the ones that have a ddof argument.

@seth-p you may have to split doc templates (or parameterize a bit more)

is their a reason shouldn't add to rolling_var,rolling_std (and the expanding versions)?

Those functions already support it (through kwargs, I believe; am not in front of computer), though it isn't documented. See in test_moments.py where they are called with ddof=0.

> On Sep 17, 2014, at 8:34 AM, jreback notifications@github.com wrote:
> 
> is their a reason shouldn't add to rolling_var,rolling_std (and the expanding versions)?
> 
> —
> Reply to this email directly or view it on GitHub.

needs rebase - give a check, I only think the docs show up for rolling_cov (not the others)

Correct. While `expanding/rolling_var/std` have supported `ddof`, it hasn't been documented (cf. #8064). Since `expanding/rolling_cov` did not support `ddof``, as part of this PR I added mention of``ddof``to their doc strings when I added the``ddof``argument to them. Still need to add mention of``ddof``in the documentation of``expanding/rolling_var/std``. Unless you prefer otherwise, I will try to do so as part of this PR, which will then also close #8064.

@seth-p ahh, yes that is good, thxs.

Rebased and added  `ddof` to the `expanding/rolling_var/std` doc strings.

looks good, ping when green

Green.

@seth-p thanks as usual!
Eliminating contextmanager based transaction-handling
State: closed

To close #8277


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

Comments:
General comment: is the added complexity in the PandasSQLTable (not fallback! so the one based on sqlalchemy) worth it? (has it added value?) 
Because the committing/rolback is something that sqlalchemy can do for us (as it did). So another option would be to leave it as is in PandasSQLTable and only adapt it in PandasSQLTableLegacy? 

This probably makes it a bit more hacky in PandasSQLTableLegacy, but keeps PandasSQLTable as we would do without having to worry about mysql-fallback-support. And when we remove that support we can just clean up the hack in PandasSQLTableLegacy, without having to clean up PandasSQLTable .

@jorisvandenbossche I think you are right -- my fix was probably overly-complicated.  Here is another possible approach: we use the sqlalchemy contextmanager for non-legacy mode, for legacy mode we create our own contextmanager using transaction handling methods.  

That seems like a nice approach!

@jorisvandenbossche  I incorporated your suggestions .  I also upgraded the requirement for the Python 2.6 CI environment to sqlalchemy 0.8.1 (it was sqlalchemy 0.7.4 previously , which didn't support the `engine.begin()` contextmanager).
Now squashing.

Are you sure about the version. If I look at that section of the docs for version 0.7, it already uses `engine.begin`: http://docs.sqlalchemy.org/en/rel_0_7/core/connections.html#using-transactions

Aha, it is added in 0.7.6. Can you change it to that version?

edit: or take 0.7.10, the latest in the 0.7 series

I can, though for some reason I thought we were standardizing on 0.8.1 ... no?

In any case, I changed the requirement to 0.7.10

For now, the 7 series was still supported (tested for), but you are right that the docs state 0.8.1. Maybe we should change that.

OK, I will merge when it is green!

@artemyk thanks!
BUG: rolling_apply(..., center=True) should not append NaNs
State: closed

Closes #8269.


Labels:
name: Bug
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations

Comments:
@stahlous, could you please review? Thanks.

@seth-p can you do a perf check

@jreback, afraid I haven't managed to get vbench running (on Windows). I can try some simple tests with `%timeit`.

right I remember that!

ok, just use the existing vbench (and timeit manually then - just the relevant ones obviously!)

report and non-trivial changes

Sorry for being dense, but I don't follow "just use the existing vbench".

@seth-p what I mean was copy-past the vbench code (for a rolling operation that is applicable here) to ipython and timeit :)

that said - if you feel the need to add a vbench go ahead (this is just a confirmation)

I'm traveling today. I'll review when I get a chance, which might be a day
or two.
On Sep 15, 2014 11:37 AM, "seth-p" notifications@github.com wrote:

> @stahlous https://github.com/stahlous, could you please review? Thanks.
> 
> —
> Reply to this email directly or view it on GitHub
> https://github.com/pydata/pandas/pull/8275#issuecomment-55636361.

Afraid I'm not quite sure about the vbench thing, but did some simple `timeit()` timing. I think the times are comparable.

```
        from timeit import timeit
        for n in [10, 1000, 10000]:
            for win in [3, 100, 1000]:
                number = 100000 // n
                print("n={:6,}, win={:5,}, number={:6,}, time={}s".format(n, win, number,
                    timeit("rolling_apply(s, {}, sum, center=True)".format(win),
                           "from pandas import Series, rolling_apply;"
                           "s = Series(1., index=range({}));".format(n),
                           number=number)))
```

master:

```
n=    10, win=    3, number=10,000, time=0.892670777595987s
n=    10, win=  100, number=10,000, time=0.7224112964259015s
n=    10, win=1,000, number=10,000, time=0.9323838644810276s
n= 1,000, win=    3, number=   100, time=0.1426375056860203s
n= 1,000, win=  100, number=   100, time=1.099650529073028s
n= 1,000, win=1,000, number=   100, time=0.023143675721702284s
n=10,000, win=    3, number=    10, time=0.13533986676669185s
n=10,000, win=  100, number=    10, time=1.2150394992196825s
n=10,000, win=1,000, number=    10, time=10.009422737328038s
```

with this PR:

```
n=    10, win=    3, number=10,000, time=0.8404246091626536s
n=    10, win=  100, number=10,000, time=0.6828260001602349s
n=    10, win=1,000, number=10,000, time=0.8512381454336704s
n= 1,000, win=    3, number=   100, time=0.14204423939089583s
n= 1,000, win=  100, number=   100, time=1.0980743344128223s
n= 1,000, win=1,000, number=   100, time=0.02169394742574715s
n=10,000, win=    3, number=    10, time=0.14353667491456878s
n=10,000, win=  100, number=    10, time=1.2034883528451288s
n=10,000, win=1,000, number=    10, time=10.038638784916468s
```

ok, looks ok. @seth-p ready?

Yep, I think so.

> On Sep 17, 2014, at 8:44 AM, jreback notifications@github.com wrote:
> 
> ok, looks ok. @seth-p ready?
> 
> —
> Reply to this email directly or view it on GitHub.

thanks!
DOC: Add notes explaining params is db driver dep. GH7573
State: closed

This is my first PR.  It's just an update to two doc strings.

Closes #7573 


Labels:
name: Docs

Comments:
Yep, that is looking good. 
We always ask to 'squash' multiple commits that belong together into one commit. You can easily do that with:

```
git rebase -i HEAD~2
```

and then replace 'pick' with 'squash' (for the second commit) (see eg https://ariejan.net/2011/07/05/git-squash-your-latests-commits-into-one/). And then you have to force push:

```
git push -f origin SQL_params_note
```

Done.  Thanks for the help.

@jnmclarty Thanks a lot!
PERF: Improve performance CustmBusinessDay
State: closed

Closes #8236 

`CustomBusinessDay` is now fast with holiday calendar and incrementing in different ways. 
import pandas as pd
import numpy as np
import datetime as dt

date = pd.Timestamp('20120101')
cbday = pd.offsets.CustomBusinessDay()
date + cbday
%timeit date + cbday
10000 loops, best of 3: 17.2 µs per loop

hdays = [dt.datetime(2013,1,1) for ele in range(1000)]
cbdayh = pd.offsets.CustomBusinessDay.from_various(holidays=hdays)
%timeit date + cbdayh
%timeit date + 2 \* cbdayh
%timeit date - 1 \* cbdayh
%timeit date - 10 \* cbday

100000 loops, best of 3: 14.6 µs per loop
100000 loops, best of 3: 19.7 µs per loop
10000 loops, best of 3: 22.7 µs per loop
10000 loops, best of 3: 22.3 µs per loop

Please check whether you agree with the way I initialize custom offsets now (using classmethod). 


Labels:

Comments:
I see the tests on several python versions failing. Is there a way to run tox locally and quickly? 

I tried using `tox --develop -e py34` but I get errors related to hashtable. I guess then the c-extensions of the different python versions are in conflict. 

replaced by #8293
BUG: DatetimeIndex.asof matches partial dates
State: closed

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

current master:

``` python
In [6]: idx.asof('2010-02')
Out[6]: Timestamp('2010-02-01 00:00:00')
```

fixed:

``` python
In [3]: idx.asof('2010-02')
Out[3]: Timestamp('2010-01-31 00:00:00', offset='M')
```

Will try to do a test_perf on this later. I've been working in python 3, but I don't think vbench is ported for that.


Labels:

Comments:
looks good
BUG: boxplot fails when one column is all NaNs
State: closed

Fixes #8181. Currently the boxplot fails when trying to compute quantiles on an empty array, which numpy can't deal with. Works okay if we use an array with a single `np.nan` instead, the empty column is just left out of the plot as expected. 

I guess that might be a bit of a hack, relying on how numpy deals with a single `np.nan` in an array versus an empty array? If people think it's too risky to depend on that behaviour I'll try to rethink.

New output:

``` python
import pandas as pd
import numpy as np
import matplotlib.pyplot as plt

df = pd.DataFrame(np.random.randn(100, 4))
df.loc[:, 0] = np.nan
df.plot(kind='box')
plt.show()
```

![missing_boxplot](https://cloud.githubusercontent.com/assets/1460294/4228586/39e763e4-395e-11e4-83d6-2f79fc12e4c3.png)


Labels:
name: Bug
name: Visualization
description: plotting

Comments:
minor comments and squash

@TomAugspurger pass to you, merge at your discretion

@TomAugspurger this one's had the issue numbers added and then squashed, OK to merge or do I need to come up with a different approach?

I think this is fine. I'll merge tomorrow when I get connected to wifi again. 

Sorry, I let this get out of date.

Merged via: https://github.com/pydata/pandas/commit/fb977d70da6b2fa798f27940db9f07f0d42fb54f

This is more properly addressed by https://github.com/matplotlib/matplotlib/pull/3571
BUG: interpolate with no nans and limit
State: closed

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

I was assuming that there would be at least one NaN when I was adjusting for the limits. Pretty simple fix. 
Here's the intended behavior:

``` python
In [1]: s = pd.Series([1., 2, 3])

In [2]: s.interpolate()
Out[2]: 
0    1
1    2
2    3
dtype: float64

In [3]: s.interpolate(limit=1)
Out[3]: 
0    1
1    2
2    3
dtype: float64
```


Labels:
name: Bug
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

Comments:
thanks @TomAugspurger
BUG: DataFrame.to_string ignores col_space when header=False
State: closed

Fixes #8230. I think this is a pretty easy fix, `col_space` wasn't being used at all when `header` was `False` so just had to add the minimum width. 

Results:

``` python
import pandas as pd
df = pd.DataFrame([[0,1,2]], index=['Sum'], columns=['A','B','C'])

print(df.to_string(col_space=20))
# Output:
                       A                    B                    C
Sum                    0                    1                    2


print(df.to_string(col_space=20, header=False))
# Output:
Sum                    0                    1                    2
```


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

Comments:
can you post the results above ? (squash too)

merged via https://github.com/pydata/pandas/commit/0d6264fd9b2f692dd62c855d1ee11ed635958926

thanks @onesandzeroes !
API: raise on setops for + and - for Indexes (GH8226)
State: closed

closes #8226 
- ToDo
  - [x] currently a raise `TypeError` (need a better message), possibly could deprecate and just warn
  - [x]  API example

Combining indexes via `+` now requires `.union()`
Diffing indexes via `-` now requires `.diff()`

These make the Indexes use setops (`&`,`|`,`^`) ONLY
numeric ops will work on appropriate Index ONLY (see #8184) (e.g. `DatetimeIndex` +/- `Timedelta` or `TimedeltaIndex` +/- `TImedeltaIndex`)
Also `Int64/Float64Index` would gain ability to `+/-` as well 

This will promote consistency that `+` means add (and not union), and `-` means subtract (and not difference), which IIRC has come up in a few issues


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

Comments:
@jorisvandenbossche 

cc @shoyer 
cc @immerrr

any addl thoughts on the PR? to summarize
- deprecating `+` and `-` for Index ops (recommening replacing by `.union()` and `.difference()`), to allow _some_ Indexes to have numeric ops (Int64/Float64 can do `*,/` now), `dti - dti` contemplated. This also promotes consistency with operations with scalars which are always numeric-like.
- replacing `.diff()` with `.difference()` (to avoid confusion with actuall `.diff()` methods in Series

No action on `|`,`&`,`^` (so its a bit inconsistent).

Looks fine to me.

ok, bombs away

@jreback seems the changes in this PR in the indexing docs are gone lost in the PR which reorganised the indexing docs: https://github.com/pydata/pandas/commit/ee00dc7297b016b5e508f30c318b5c57a4c024e5, https://github.com/pydata/pandas/pull/8204

you mean this? http://pandas-docs.github.io/pandas-docs-travis/whatsnew.html#whatsnew-0150-index-set-ops

oh i c, in the indexing.rst docs themselves. ok fixing up now.

@jorisvandenbossche #8485 should cover
DOC: fix formatting error in io.rst
State: closed

Lines had an extra space causing the code block above to keep going for the rest of the note.

Before:
![selection_020](https://cloud.githubusercontent.com/assets/1284973/4203651/f3b0d9c4-3830-11e4-9db9-20e418cca8dd.png)


Labels:

Comments:
thanks!
API: Series.to_csv(path=None) should return string to match DataFrame.to_csv()
State: closed

Addresses #8215. Currently, `Series.to_csv(path=None)` returns `None`. The `DataFrame` method returns a string, and already had this behaviour mentioned in the docstring, so it should be OK to make the `Series` method match this.


Labels:
name: Bug
name: API Design
name: IO CSV
description: read_csv, to_csv

Comments:
minor comment. pls squash. ping when green

@jreback Added those comments and squashed, should be good to go.

@onesandzeroes thanks!
ENH to_latex mi index will use & sep for levels
State: closed

follow up of #7982

_One question I had was the use of `{ }` (is that a convention? I removed it and it seems to compile ok.)_

Note: should test MI cols and MI index. Ah cripes... the names of the column Index don't print, not did they before...


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

Comments:
merge when you are happy witht his

eeep, will have to fix a bug I found in this/`to_latex` (with index.names and column.names)!
ENH: sql support for Timestamp (GH7103)
State: closed

Superceded by #8208 (converting to object dtype also converts the datetime64 values to datetime.datetime, so it is no longer needed that sqlalchemy can work with pandas' Timestamp type.

---

Closes #7103, #7936

This adds a `pandas.io.sql.Timestamp` class to handle `Timestamp`s in `to_sql`. This basically converts it to a `datetime.datetime` before writing to the database.

Problem is that this makes it slower (I tested it with sqlite, and it's for a dataframe with only a datetime64 column about 20% slower), while it did already work before for some drivers (like psycopg2 and MySQLdb).


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

Comments:
so just do this for certain database types then (will prob be similar for timedelta)

It depends on the driver, not the database type. But indeed, we could do that, but that feels a bit clumsy. Also, I tested it with psycopg2/pymysql/MySQLdb/mysql.connector, but there a lot more drivers for which I don't know the behaviour (and also not for different versions of the drivers).

While testing the fix for NaN values, I noticed that doing `df.astype(object)` (needed to get in None values, and not NaN) also converts `Timestamp`/`datetime64` to `datetime.datetime`. Is this the expected behaviour? I would have expected individual `Timestamp` objects.

@jorisvandenbossche that is expected. the `object` dtype is a `ndarray` of `datetime` objects (I guess for compat reasons).

You _might_ want to preconvert any datetimes/timedeltas at the start (iow, separate the frame into various 'blocks'), which you then iterate all together. Don't try to concat (or they will be re-coerced)
And to be honest you can simply do this for types that need `NaN` -> `None` as well, e.g. drop down to numpy object arrays (or rec-arrays). Might be a bit of work at first, but then you can easily do what you need quickly. I actually do this for `PyTables`, see [here](https://github.com/pydata/pandas/blob/master/pandas/io/pytables.py#L3664), (and the next method, where I create a structured/rec array), and fill with already coerced values (e.g. `datetime64[ns]` have already by tz converted and are now `int64` and such, strings are already an appropriate dtype, etc.)

Superceded by #8208
DOC: reorg indexing docs
State: closed

DOC: created advanced indexing section
DOC: add internals section (GH4082)

Mostly just splitting up the indexing docs, but I reorded / revised the Multiindex sections
a bit to avoid duplication / make more clear.


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

Comments:
@jorisvandenbossche pls have a look

![pandas powerful python data analysis toolkit pandas 0 14 1-354-g12248ff documentation](https://cloud.githubusercontent.com/assets/953992/4180278/21517dca-36f1-11e4-94e3-c3abeabf991c.png)

![multiindex advanced indexing pandas 0 14 1-354-g12248ff documentation](https://cloud.githubusercontent.com/assets/953992/4180279/2b887668-36f1-11e4-9c0b-0ab32eca28a6.png)

+1 on splitting it up a bit. I have wanted to do that for the rest of the docs also. I personally think it would even be better to also have a more nested hierarchy, but the current html theme does not really support that.

I will try to take a more detailed look tomorrow on the docs itself.

@jorisvandenbossche gr8 thanks.

lmk after u have reviewed this

@jreback I went through it, some comments (probably mostly on the content that was already there but just moved, but a good occasion to check that). For the rest, certainly fine to merge!