BrandonFors/pandas-github-issues huggingface dataset:
Added handling for v3 advanced segment ids which aren't just ints as of July 15th
State: closed

The old int based advanced segment ids are deprecated. The new segment ids are alphanumeric. I know from experimentation that their form includes alphanumeric and alphanumeric+hyphen. Updated code for both
cases. Reference: https://developers.google.com/analytics/devguides/reporting/core/v3/changelog


Labels:

Comments:
pls put these tests in the existing test_ga.py file
pls hook up Travis (see contributing.md)

Ok. Updated tests... will check on Travis.

OK. Travis build passed as well.

Can you this down to one commit and rebase on master?

Also, does this break v2 support? (or is v2 no longer supported by Google?) 

Sure! How can I do the squash given the current state of my branch? Do I need to issue a new pull request?

As far as v2 goes this does not break v2 support. There are unit tests to support those cases as well in my code.

Don't need a new PR, just squash and do git push --force

Ok. All set!

@jcbozonier one last thing - can you add a note to doc/source/release.rst that says you added support for v3 segment ids for GA?  And then just do `git commit --amend -C HEAD` (amends your previous commit and keeps message) and push one more time, then I'll merge.

as an aside if you would like to they the exception messages when deps are not present and make a bit nicer 

Added a note. Also, I wanted to add a nicer exception but I wasn't sure how to do it in a way that was 2.6 AND 3.2 compatible. The next enhancement I make I'll take another stab at it.

compatible exception is just:

`raise ExceptionName("My error message")`, whereas <= 2.6 syntax was `raise ExceptionName, "error message"`. Is that what you mean?

the former version is 2.6 - 3.X compatible

Yeah I think so.

Does it make more sense just to let it blow? Why even catch those exceptions?

it's basically just something like:

``` python
try:
   import <your dep>
except ImportError as e:
   raise ImportError("read_ga requires Google's ga package: %s" % str(e))
```

Your call - I'm +1 if you want to add a nice error message but I'll merge this after you make that slight to change to the docs note if you don't want to add it.

I'm fine with adding that. To clarify, you mean to the split test file, right?

err unit test*

No - I think @jreback meant in the `read_ga()` method

I guess you'll need to rebase this - apparently there's a merge conflict with something else (probably another PR that was merged in the interim)

Rebase done.

Thanks!
BUG: Fixed issue #5156: segfault on read_csv
State: closed

closes #5156 

Our c parser was not checking array bounds. 

Example:

```
In [1]: from StringIO import StringIO; import pandas as pd

In [2]: data = """\     
1,2,3                      
4,,6
7,8,9                      
10,11,12\n"""

In [3]: df = pd.read_csv(StringIO(data), header=0, names=['a', 'b', 'c', 'd'], engine='c')
CParserError: Too many columns specified: expected 4 and found 3
```

Previously, this would segfault.


Labels:

Comments:
thanks!  helpful as always
TST: test_fast.sh and test_multi.sh should skip network tests
State: closed


Labels:

Comments:
@jreback @jtratner @y-p any objections? totally minor, but if someone is using these, `@network` tests will be run.

ok

back to the way it used to be. good.

@y-p FWIW, it was only ever that way because `@network` tests were also marked as `@slow` which results in such tests not being run on Travis.
DOC: changed the term `Vern` to `Versions` to be clearer
State: closed

based on this SO question: http://stackoverflow.com/q/19438309/1301710


Labels:

Comments:
You need to either change the column widths or auto-detect (remove them); pls build the docs and see which works better

@jreback I changed to "version" and updated "witdth". you can look at the result as github renders rst-files.

@jreback 

Sorry that was not intended! Must have been a problem with github only editing (I should work more carefully).
I reopened the same pull request, but created a local branch and add the changes, see

https://github.com/pydata/pandas/pull/5277
DOC: core/generic/drop docs from array-like to list-like
State: closed

related #5248


Labels:

Comments:
cc @ruidc better? (I know its only 1 word.......)

if it works, it works, but to me is not any clearer.

still unclear

can you suggests doc/code change to make more clear?

what about: 
"passing a string or tuple will be treated as a single index label" ?

sure...

I thought the confusing bit of the doc is that it's not clear that one can pass a single label. So why not this:

labels: scalar or list-like

Or whatever would be appropriate to designate a single label

.... as indeed single integer labels are treated the same way, so it's not just tuples and strings that are treated this way - but I haven't delved beyond the commits today to understand _when_ this happens

How about: single label or list-like?

that's much clearer indeed!

y

pushed via 97f5878003116ea463545456858f049b38107a8f
ENH: Add inplace option to drop and dropna
State: closed

And appropriately invalidates caches as necessary BUT _still_ makes a copy.  @jreback and I decided that it shouldn't call `__finalize__` a second time in `_update_inplace` (so if metadata is supposed to change, it's not going to be updated, but subclass can always override it). Closes #1960, related #2325 (doesn't actually fix because still makes a copy).

Adds `_update_inplace` method that takes a result and invalidates the internal cache then reindexes.

---

@jreback - would you mind taking a look? It's almost there, I think I'm
missing one cache reference or something... (or I'm just using reindex
wrong).


Labels:

Comments:
I'll have a look tomorrow

ping @jreback.

@jseabold - I think this covers what you were thinking. Figured it was better to just stick with the inplace idiom.

Updated the note to reflect that this still copies, it just updates the wrapper rather than changing the internals in place, so doesn't avoid "2X" problem referenced in #2325.

@jreback you okay with this now?

ok
Make doc compiling under windows workable
State: closed

The doc generation failed under windows due to problems with sphinx
and encoded umlauts in code (see links in
https://github.com/pydata/pandas/issues/5142).

The workaround is to replace the offending text with one which does not fail
(but which makes the example a bit pointless), build the docs and restore the
old text.

Closes: #5142


Labels:

Comments:
2 things:
1. Can you add a comment to the doc fixing code that says 'Sphinx fails on embedded unicode on Windows'.
2. Can you make the comment show up only on Windows? Eg put it with the replacement? I think it would be cleaner.

@jtratner like this?

Please remove this PR, I think I found the real cause of the failing doc builds
Fixed assertion error message
State: closed

This is a pretty straightforward fix, message was simply inverted.


Labels:

Comments:
thanks!

did you actually see this error? how?

@jtratner It came up as part of a traceback in some code I was debugging... Was a pretty weird error to try to make sense of: 

```
AssertionError: Truncate: 2012-10-02 14:51:00 must be after 2012-10-02 14:50:50
```

Err... Yeah? ;)

@ojh can you post code that generated it? maybe need a test for it......

The reason we're asking is because if you see an AssertionError, the logic
is probably screwed up somewhere upstream (i.e., some function isn't
checking for bad input or is passing arguments wrong). Generally, users
should never see assertion errors.

@jreback @jtratner Sorry for the slow response, it was late when I posted this last night, and I've been busy all morning today...

The code I was dealing with is a bit hairy, but the following should illustrate the issue clearly enough:

``` python
import numpy, pandas

rng = pandas.date_range('1/1/2011', periods=72, freq='H')
ts = pandas.Series(numpy.random.randn(len(rng)), index=rng)

ts.truncate(before='10/31/2011', after='12/31/2011') # No problem here

ts.truncate(before='12/31/2011', after='10/31/2011') # Switched dates, to illustrate
```

That last line then throws the following error (which is pretty much what I would expect, except that the order of the dates is wrong in the message):

```
AssertionError: Truncate: 2011-12-31 00:00:00 must be after 2011-10-31 00:00:00
```

The author of the actual code which resulted in this had assumed that the dates he was reading in would always be in chronological order - turns out this wasn't the case in one particular CSV file that got imported... Obviously, this has now been fixed with an appropriate failsafe added. I fully agree that users should never see assertion errors, but the confusing error message made the issue harder to debug for me, so I figured it might save someone else the confusion in future.

Thanks - that's helpful
CLN/ENH: Make names getattr'able in core/ops +PEP8.
State: closed

Now the name passed to a method can be used to get that method from the 
NDFrame (previously you had things like `or_ [|]`. better to just have them
all be named exactly what they are s.t. subclasses can do
`getattr(super(self, MyClass), name)(*args, **kwargs)`.


Labels:

Comments:
looks  fine
CLN: Add _reset_cache method to PandasObject
State: closed

I think this is the right way to do it. MI needs this for _engine and
friends. I think there are other objects that use `cache_readonly` that
might want to reset too.

I also moved the local dir methods off of StringMixin to PandasObject
(where they ought to have been).

Fixes #5236 and fixes #5225.


Labels:

Comments:
this is fine

FYI - I realize that am using _cacher in NDFrame for the ref to the _item_cache

but that's different anyhow

`_reset_readonly_cache`?

no this is consistent with the _cache holder

(but I may change the other one now that I think about it)
BUG/TST: Fix Excel writers with duplicated column names.
State: closed

Test and fix for issue #5235


Labels:

Comments:
@jreback Added comment with issue number.

gr8....also need a release notes entry.....thanks...then can merge

@jreback Added the following to `release.rst`:

``` rst

  - Fixed bug in Excel writers where frames with duplicate column names weren't
    written correctly. (:issue `5235`)
```

If you wish to reword let me know and I will amend.

@jreback  Fixed `:issue:` typo.

gr8 waiting for Travis

then will merge

thank you for the PR!
FIX: OLS.predict() - cast input to DataFrame from Series before performing operations
State: closed

Closes #5233.  Simply cast to a Series to a DataFrame before dropping NAs or filling.


Labels:

Comments:
can u add some tests that demonstrate the issue?

Frankly, I think the better fix (separate from this PR) would be to have Series' dropna match signature of DF and Panel, since both 'any' and 'all' or thresh apply. Just means that you don't have to care about what kind of NDFrame you have.

Please update your commit message to emphasize that it's only in OLS-Predict. I read the title and was quite confused.

@jreback @jtratner Here is the issue demonstrated and I put this as a test in the last commit that now passes with the fix.  I agree with @jtratner that series and dataframes should have the same drop arguments.

```
y = tm.makeTimeSeries()
x = tm.makeTimeSeries()
model = ols(y=y, x=x)
model.predict(x=x)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File ...ols.py, line 446, in predict
    x = x.dropna(how='any')
TypeError: dropna() got an unexpected keyword argument 'how'
```

looks fine to me.

closing in favor of #5251 (credited for your tests!)
BUG: Groupby filter maintains ordering, closes #4621
State: closed

Simply adds `np.sort`.

Includes tests for SeriesGroupBy and DataFrameGroupBy, using indexes that are sequentially increasing, sequentially decreasing, and shuffled. Also, this PR removes all ad hoc sorting involved in the original filter tests -- now no longer needed. Much better!


Labels:

Comments:
looks good
in theory u might want to have a kw
sort=True that you would accept in order to turn this off
(only would matter with. large number of groups though)

can u add a release notes entry as well

I'll add the sort keyword. Does that qualify as an API change, or is it too minor?

pretty minor - just make sure doc string is good

Ready (assuming Travis agrees!)

This looks good, though I dislike the use of `sort` kwarg to mean "maintains original order"... not sure I have alternative.

Yes, I agree - that's very confusing. Is the alternative not sorting it?

sort is the kw in groupby already so it's consistent (or maybe it's sorted?)
I don't remember u may be able to just use that kw

@jreback that's a good point, I'd not seen or used that.

>  sort : boolean, default True
>  Sort group keys. Get better performance by turning this off

Although this is somewhat different, in that case it is actually sorting the keys. Here we are putting them back into their previous order of the index (and this may - or may not - be sorted)...

Exactly. IMO, there is a strong case for not even allowing the option to skip the sorting. For one thing, it's difficult to test, which seems like a red flag. (My original tests sorted the result before comparing to the expected one.)

this is the indexer it should always be sorted
just skip a keyword on this

it should always be in the same order as the groups which is why it needs sorting, because the groups are sorted (groupsort_indexer)

Removed sort, including docstring and release notes. Ready.
fix business day calculation
State: closed

This will fix the so far broken calculation of business day periods and the corresponding tests reported

closes #5203.

The expected behaviour found in the present tests is that the date is always rolling forward to the next business day:

```
Friday:   Period('2012-3-09', freq='B') => '2012-03-09'
Saturday: Period('2012-3-10', freq='B') => '2012-03-12'
Sunday:   Period('2012-3-11', freq='B') => '2012-03-12'
Monday:   Period('2012-3-12', freq='B') => '2012-03-12'
```

Can anyone confirm this?


Labels:

Comments:
For reference, what do any other periods do when the date passed in does not actually match the desired frequency. 

@wuan thanks for the PR!
BUG/CLN: MI now checks level & label compatibility
State: closed

Via verify_integrity kwarg. Internals all pass False.I don't think I've missed anything / passed verify_integrity=True where
it ought not to be.

Fixes #5213.

Clearly this adds a (likely small) perf hit - but should only occur
from external MI calls that specifically call the constructor with
levels and labels (for example, doesn't occur with `from_arrays` or
`from_tuples`). For 0.14 I'll have to do it differently, but this should
work for 0.13.


Labels:

Comments:
Changed it to validate pickles too.

people okay with this? @wesm?

good by me

Performance impact probably minimal so this is fine to go in. 

Okay I'll rebase. I might add something to the docs about how setting levels or labels triggers a new round of validation.
ENH: Add usecols option to python parser.
State: closed

Closes #4335

This ticket felt a little bit like whack-a-mole. 

The key is that names can have two meanings: if it matches length of usecols then it is the name to apply to the (numeric) usecols. If it matches the length of the original columns, it is the set of column names and string values in usecols refer to it. 


Labels:

Comments:
pls also add a release notes entry and update io.rst docs (e.g. the default for the header is now slightly different as you changed in the doc string?)

for py3, use `compat.string_types` instead of `basestring` (you may also need to import it)

I added release notes. @jreback do we have to update the io.rst docs manually or is that generated? 

FWIW my docstring update reflects pre-existing behaviour. I just made it explicit.

@guyrt you can edit io.rst (or any other doc file) just like code....edit and it will be included in the commit
the docs themselves are generated from that...(which you can build to look at locally if you would like).

can you rebase and squash?

Looks like I goofed on the rebase.

edit: fixed

merge this? (as its separate from #5156) ?

This one is ready to merge. #5156 will be a separate PR
CLN/BUG: Better validation of levels/labels/names
State: closed

Improved validation of levels, labels and names (plus fleshed out test
cases). Fixes #4794.

Changed an assert_almost_equal to assert_series_equal to better reflect
actual return value.

Also makes set_levels, set_labels and set_names match behavior of rename to _not_ return self if inplace.


Labels:

Comments:
look ok to me

took a bit of wrangling to test correctly given the slightly weird internal representation of levels and labels. Do you think this needs to test that levels are all integers? I feel like most of the time people should not be calling MI directly.

note that 0.12 and earlier wasn't doing that either.

u could prob validate that it's int64 dtype 
I agree users shouldn't really touch mi internals 

I wonder if I can just call Int64Index and that'll be good enough? I'll try

I guess there are internal pandas methods that may create labels/levels (that re not in index) - eg reshape/ concat

huh? but I think the internal representation depends on levels being integers no? It's really simple actually, just change `_ensure_index` to `_ensure_int64index`. Going to see what happens.

of course it's labels that's actually supposed to be integer :P.

I'm going to leave it - there's a lot of complexity with checking that it's not a float and such, given that it's internal-ish (and we don't really have a good fastpath yet), probably better to just not figure out something complicated to check. Unless you feel strongly, I'll leave it for 0.14 and the index refactor (where it should be easier to reason about and check).  Internal calls all use `_ensure_int64` anyways.

@jreback okay?

yep
DOC: make api docs reflect api usage (#4539)
State: closed

Closes #4539.

This removes all use of `.. currentmodule:: pandas.some.submodule` (in favor of `.. currentmodule:: pandas`) in api.rst. Rationale is that docs should reflect the user api usage.

Only exception are `pandas.io.sql.read_frame/write_frame` and `pandas.io.stata.StataReader` methods, because they are not available in the top-level pandas namespace.

All old links will remain valid because they are still generated because I added all those in a comment at the bottom in api.rst (only not for Index, because this was only added very recently in api.rst).  
The **downside** with this approach is that you now get warnings when building the docs (`document isn't included in any toctree` for these functions). I am not sure how I can prevent this while keeping the old links but not including it visually in api.rst.


Labels:

Comments:
OK, solved the downside by adding a hidden toctree. So now all old links remain valid, and no warnings are generated. The only downside is that there is a big chunk of lines at the bottom in api.rst that is maybe somewhat noisy, just to keep the links.

If you can add comments, can you leave a note saying that its' there to maintain links?

I did, quoting from the commit 

```
HACK - see github issue #4539. To ensure old links remain valid, include
here the autosummaries with previous currentmodules as a comment and add 
them to a hidden toctree (to avoid warnings):
```

If it is not clear enough, please suggest something.

Travis had a failing (related to sql: `test_basic (pandas.io.tests.test_sql.TestMySQL)`. Strange, because it is not related to the docs, and before I updated the commit (a forced push), the commit was green. Is this a problem?

yep unrelated
API: rename _prop_attributes to __finalize__ (in NDFrame)
State: closed

- rename _propogate_attributes to _metadata (in NDFrame)
- change methods which directly pass metadata to use **finalize**

These provide a simple mechanism for meta data propagation, which
can be overriden in subclasses if desired. Currently these only do non-ambiguous 
propagation.

makes #60, #2485 easier


Labels:

Comments:
@jtratner 

I was actually toying with the idea of having `__finalize__` have a signature like this:

```
def __finalize__(self, other, method=None):
.....
```

then a sub-class (or even in the current implementation of ``__finalize__`) you could dispatch on the method (which would be the name of the calling method) and do something interesting

that's similar to what numpy does. Let's be clear about lifecycle for NDFrame:

`__new__` --> not used (just object's `__new__`)
`__init__` --> called to reconstruct object, no access to metadata of previous
`__finalize__` --> called after `__init__` is done initializing after things like arithmetic, sorting, and slicing that generate either same type _or_ constructorsliced?

So if you do `df["A"]`, would that call `__finalize__`? what about `df.loc` or other structures like that? (or the methods that promote upwards)?

`__init__` doesn't return anything, should `__finalize__` return self?

also which `__finalize__` gets called on `df["A"]`? I'm assuming it's whatever is on the sliced constructor?

I'm a fan of having it return self, looks nicer in the code too.

btw - to keep in mind, from the Python docs on magic methods:

> `__*__`
> System-defined names. These names are defined by the interpreter and its implementation (including the standard library). Current system names are discussed in the Special method names section and elsewhere. More will likely be defined in future versions of Python. Any use of `__*__` names, in any context, that does not follow explicitly documented use, is subject to breakage without warning.

http://docs.python.org/2/reference/lexical_analysis.html#reserved-classes-of-identifiers

I'm guessing that numpy is popular enough for them not to worry about it :smile: 

I still like `__finalize__` though.

Does this have a performance hit? I'd guess that it would be minimal.

it's already in there (and minimal)
just per construction
CLN/BUG/ENH: Raise AttributeError with pd.options
State: closed

Fixes #5182, closes #5186.

It's called OptionError now and it subclasses both AttributeError (for
hasattr) and KeyError (for backwards compatibility). Hurray!


Labels:

Comments:
Anybody object to `OptionError(AttributeError, KeyError)`? Backwards _and_ forwards compatible -- whee!

Exception abuse! but ok! practical beats purity!

this is fine for #5182, but I don't think we should fix for ipython 0.2-dev when they are at least a year away and this interface could change

but maybe lodge an objection as an issue there? (or at least an incompat change they are doing)...?

yeah, but generally it's surprising to have attribute access raise KeyErrors anyways.

@jtratner yep...that's why agree for #5182 it makes sense
fix replacement key for deprecated option `display.height`
State: closed

replacement key for deprecated option `display.height` was set to `display.height` itself. I believe it should be `display.max_rows` instead.

This avoid a weird message in `describe_option()` :

```
display.height: [default: 60] [currently: 60]
: int
        Deprecated.
    (Deprecated, use `display.height` instead.)
```

(I spotted it while reading the online doc http://pandas.pydata.org/pandas-docs/dev/basics.html?highlight=display.height#working-with-package-options )


Labels:

Comments:
this seems right....

@jtratner @cpcloud @y-p ?