BrandonFors/pandas-github-issues huggingface dataset:
REF: gradually move ExtensionIndex delegation to use inherit_names
State: closed


Labels:
name: Performance
description: Memory or execution speed performance
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
Hello @jbrockmendel! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:









There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-01-27 19:49:05 UTC
Closing to clear the queue, will revisit once indexing fixes are done
Are all of these changes appropriate for 1.0.0? Or does it rely on stuff that hasn't been backported?
> Are all of these changes appropriate for 1.0.0? Or does it rely on stuff that hasn't been backported?

`inherit_from_data` was in the RC, though this does change that a little bit.  I could do a PR that omits the CategoricalIndex and TimedeltaIndex edits.
thanks @jbrockmendel
REF: Move generic methods to aggregation.py
State: closed

xref #29116

https://github.com/pandas-dev/pandas/pull/29116#pullrequestreview-340272465

based on @jreback comment, this PR is a precursor PR for #29116 to make PR smaller and more readable

Labels:
name: Refactor
description: Internal refactoring of code

Comments:
@jreback could you take a look? pls lemme know what changes you want to see here? and then I could start finishing up #29116 after this is merged. Thanks!
Hello @charlesdong1991! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:









There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-01-20 22:05:57 UTC
lgtm. ping on green.
thanks @charlesdong1991
CLN: remove pydt_to_i8
State: closed

It's only used in one non-test place, and all supported python versions now support datetime.timestamp, so we can use that directly.  Its a little bit less performant, but not used often.

Labels:
name: Timedelta
description: Timedelta data type
name: Clean

Comments:
@jbrockmendel can you merge master?
rebased+green
thanks
BLD: Run flake8 check on Cython files in pre-commit
State: closed

- [x] closes #30843
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: CI
description: Continuous Integration
name: Code Style
description: Code style, linting, code_checks

Comments:
@datapythonista Fixed the configuration, CI now passes.
> Looks good, I'd probably have the flake config files somewhere else, since we've already got too much in our root, but other than that looks good.

I think a `flake8` directory would make sense.  Would also help to simplify filenames.
`flake8` or `.flake8`?
`flake8` (no dot) should be fine I think. Keep things simple.
@jbrockmendel can you try this out
I'm missing something from the workflow.  I've checked out this branch, added a flake-violation to groupby.pyx, but commiting it doesnt seem to cause any problem.  What am I missing?

Can W293 for trailing whitespace be added?

Not a deal-breaker, but I like the more verbose way we list codes in statsmodels: https://github.com/statsmodels/statsmodels/blob/master/setup.cfg#L73
> I'm missing something from the workflow. I've checked out this branch, added a flake-violation to groupby.pyx, but commiting it doesnt seem to cause any problem. What am I missing?
> 
> Can W293 for trailing whitespace be added?
> 
> Not a deal-breaker, but I like the more verbose way we list codes in statsmodels: https://github.com/statsmodels/statsmodels/blob/master/setup.cfg#L73

The obvious one:
Did run `pre-commit install` once?

Otherwise: Can you tell me what you did change? Then I can try to reproduce locally.
rebased and CI passed.
>  Did run pre-commit install once?

Looks like `brew install pre-commit` followed by `pre-commit install` did the trick, thanks.

LGTM

W293 can be follow-up.
Thanks @xhochy
BUG: Handle nested arrays in array_equivalent_object
State: closed

- [x] closes #30841
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
Thanks!
CLN: remove _to_M8
State: closed

a couple of recently-merged PRs removed the last usages of this function

Labels:
name: CI
description: Continuous Integration
name: Clean

Comments:
Thanks, lgtm.  Would be nice to merge on green since there is a linting error related to this on `master` that's breaking CI.  I opened #30837 to fix it but this is more complete so I'll close that in favor of this.
BUG: added missing fill_na parameter to DataFrame.unstack() with list of levels
State: closed

- [x] closes #30740
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


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

Comments:
Hello @tanmay1618! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-01-09 18:09:40 UTC
Thanks for the PR. Does this fix a particular issue? If so will want to add tests first and foremost
Yes, it fixes this issue. #30740. Sure, I will add tests for this.
@tanmay1618 Thanks for the PR. generally lgtm. can you add a whatsnew under bugfixes.
getting this error on running tests
```
pandas/__init__.py:54: in <module>
    from pandas.core.api import (
pandas/core/api.py:4: in <module>
    from pandas._libs.missing import NA
E   ImportError: cannot import name 'NA' from 'pandas._libs.missing'
```
@tanmay1618 you probably need to rebuild the C extensions. `python setup.py build_ext -i`.
@simonjayhawkins Thanks for all the help, change requests and helping out through the PR. The checks are all green, now.
@TomAugspurger Thanks for all your suggestions and help as well.
Thanks @tanmay1618!
DOC: Fix indentation in docstring example
State: closed

- [x] closes #30830
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Docs

Comments:
Thank you :) 
@jorisvandenbossche
CLN: Removed "# noqa: F401" comments
State: closed

Implemented ```__all__``` for each changed file

- [x] ref https://github.com/pandas-dev/pandas/pull/30828#discussion_r364500688
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


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

Comments:
nice. thanks @MomIsBestFriend
DOC: Added a single '-'
State: closed

Feels weird to open a PR just for that, but I must get this fixed ;)

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

Comments:
Thanks @MomIsBestFriend 
> Feels weird to open a PR just for that, but I must get this fixed ;)

Totally reasonable.  FWIW i keep a "collect" branch going where i put miscelleneous things like this that pile up until it seems big enough to warrant a PR
> > Feels weird to open a PR just for that, but I must get this fixed ;)
> 
> Totally reasonable. FWIW i keep a "collect" branch going where i put miscelleneous things like this that pile up until it seems big enough to warrant a PR

I'm gonna remember that, thank you @jbrockmendel !
STY: absolute imports in __init__ files
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


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

Comments:
I was just about to ping :)
Thanks @MomIsBestFriend 
Personally, I am -1 on those changes (or https://github.com/pandas-dev/pandas/pull/30827). 
@MomIsBestFriend before you do any more of such PRs on absolute imports (although I don't know if there are cases left?), can we first have a discussion about this?
@jorisvandenbossche I actually did want to discuss this, so I opened [this PR](https://github.com/pandas-dev/pandas/pull/30822) which as you can see has only two changes. And when it got merged I saw it as an approval to go and change even more files.

Anyway, I don't mind reverting #30822, #30827, #30828.
Until this topic is fully discussed.
BUG: DTI.searchsorted accepting invalid types/dtypes
State: closed

Analogous to #30763 which fixed the same issue for PeriodIndex.  After this will be a PR to fix TimedeltaIndex, then one to move the fixes up to the EA methods (and share searchsorted).

Labels:
name: Datetime
description: Datetime data dtype
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves

Comments:
If a plain `list` is passed should there be an attempt to infer the dtype (or just coerce to an array-like) to determine if the `list` is valid?  E.g. should the following succeed?
```python
In [1]: import pandas as pd; pd.__version__
Out[1]: '0.26.0.dev0+1685.g9565b5666'

In [2]: dti = pd.date_range("2020", periods=3)

In [3]: dti.searchsorted(list(dti))
---------------------------------------------------------------------------
TypeError: searchsorted requires compatible dtype or scalar, not list
```
> If a plain `list` is passed should there be an attempt to infer the dtype (or just coerce to an array-like) to determine if the `list` is valid? E.g. should the following succeed?
> 
> ```python
> In [1]: import pandas as pd; pd.__version__
> Out[1]: '0.26.0.dev0+1685.g9565b5666'
> 
> In [2]: dti = pd.date_range("2020", periods=3)
> 
> In [3]: dti.searchsorted(list(dti))
> ---------------------------------------------------------------------------
> TypeError: searchsorted requires compatible dtype or scalar, not list
> ```

i suppose we could wrap list-likes with pa.array, but this would be an enhancement; can you open an issue.
thanks @jbrockmendel
TST: Added 'match=' to some bare pytest.raises
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Testing
description: pandas testing functions or related to the test suite

Comments:
Thanks @MomIsBestFriend
BUG: BooleanArray.value_counts dropna
State: closed

Closes https://github.com/pandas-dev/pandas/issues/30685

Labels:
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
LGTM pending green
OK, updated {String,Integer,Boolean}Array.value_counts to return a nullable int64 dtype.

I moved BooleanArray.value_counts and IntegerArray.value_counts to the base class.
let me clarify my comments

i think the index should be nullable ints as that preserves the intent here
but agree the values should be int64 - this is a count after all
Thanks for clarifying.

The index can't be nullable yet. @jorisvandenbossche thoughts on the values dtype?
> Ideally we'd retain the dtype in the value_counts().index, and it seems like we're saying here "well we cant do that, so let's shoehorn the dtype into the values"

IMO, ideally we retain the nullable dtype in *both* the index and the values. But indeed, for index we can't do that yet, so for now it's only the values.

>  It seems weird that the values would be anything other than np.int64

We are still returning an int dtype. But in the "nullable dtype universe"-subsystem, the "int64" dtype is the nullable int64 dtype.



So I think that's some +1s and a -0 (https://github.com/pandas-dev/pandas/pull/30824#discussion_r364862869). Merging in an hour or so unless @jbrockmendel objects.
No objection here.
Thanks.
Update NA repr
State: closed

Closes https://github.com/pandas-dev/pandas/issues/30415

```python

In [2]: df = pd.DataFrame({"A": pd.array([1, 2, None])})

In [3]: df
Out[3]:
      A
0     1
1     2
2  <NA>

In [4]: df.A
Out[4]:
0       1
1       2
2    <NA>
Name: A, dtype: Int64

In [5]: df.A.array
Out[5]:
<IntegerArray>
[1, 2, <NA>]
Length: 3, dtype: Int64

```

I think adding color with ANSI codes / custom HTML formatting is still worth doing as in #30778, but this is an improvement for now.

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

Comments:
I had to include `<NA>` in the list of default NA values for read_csv to get this working. Probably OK to do. Also maybe simplified ExtensionBlock.to_native_types, but may have broken something along the way.
On board with this once green
some doc-test failures, otherwise lgtm.
I think you need to update the tests if a nullable integer / bool / string array appears in Series/DataFrame? (or does this not work yet)?
It works. We have smoke tests in the base class. Will add some dedicated ones.
Passing now. Merging in an hour.
> I had to include <NA> in the list of default NA values for read_csv to get this working. 

Can you explain why? Pandas does not write such format to CSV files, so it shouldn't be needed for a roundtrip
API: Limit assert_*_equal functions in public API
State: closed

As discussed on today's call. Just adding `assert_extension_array_equal` relative to 0.25.

Labels:
name: API Design

Comments:
LGTM
Looking at the `assert_extension_array_equal` code, we probably want to dispatch from there to the more specialized asserts for our known extension arrays (categorical, interval, datetime, etc).

Also, do we want to have an `assert_array_equal` that can handle both extension arrays and numpy arrays?
CLN: de-duplicate boxing in DTI.get_value
State: closed

Also add a dedicated test or it to hit currently un-covered case of np.datetime64 key.  That case works in master, but doesn't go through the expected code path

Labels:
name: Datetime
description: Datetime data dtype
name: Clean

Comments:
@jreback @jorisvandenbossche related to this PR I'm trying to de-duplicate the localization going on in _get_value_maybe_box.  Back in #17920 it was decided to localize naive-looking strings when doing lookups on a tzaware DTI, but I'm surprised that we are doing the same for datetime objects (and AFAICT also np.datetime64, but we dont test that case)

The relevant test is in tests.series.indexing.test_datetime, pulling out the pertinent part:

```
rng = date_range("1/1/1990", periods=N, freq="H", tz="US/Eastern")
ts = Series(np.random.randn(N), index=rng)

[...]
# repeat all the above with naive datetimes
result = ts[datetime(1990, 1, 1, 4)]
expected = ts[4]
assert result == expected
```

When I change the behavior of get_value to _not_ cast tznaive datetime objects, this lookup raises (which I thought was the correct behavior, but apparently not?).  Looking at the blame for this test, it has been around for a long time.

Changing this would be non-trivial because it turns out that getitem[scalar] goes through DTI.get_value but slicing and setitem etc go through different paths.

I guess there's not really a question here, just opening it up for comments.
about your comment: https://github.com/pandas-dev/pandas/pull/30819#issuecomment-572281006

yeah this is the implict conversion of *strings* only to a local timezone when they are naive (the strings) and the indexing is on timezone aware indexes. note that this does *not* apply to timestamps which must be explicityly timezoned.

seems reasonable, but feel free to open an issue if more questions.

ideally we should try to limit the indexing paths, but there is a lot going on so not sure how easy this is.
thanks
REF: share comparison methods between ExtensionIndex subclasses
State: closed


Labels:
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.
name: Index
description: Related to the Index class or subclasses

Comments:
@jschendel looks like Interval comparisons are failing here.  Is that the expected behavior?
Yeah, inequalities haven't been implemented yet, and look a little buggy at the `Interval` level.
thanks
replacing '.format' with f-strings in some test files
State: closed

- [X] contributes to #29547
- [ ] tests added / passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

One format that I left untouched in `pandas/tests/util/test_assert_frame_equal.py`:

```
def test_frame_equal_unicode(df1, df2, msg, by_blocks_fixture, obj_fixture):
    # see gh-20503
    #
    # Test ensures that `tm.assert_frame_equals` raises the right exception
    # when comparing DataFrames containing differing unicode objects.
    msg = msg.format(obj=obj_fixture)
    with pytest.raises(AssertionError, match=msg):
        tm.assert_frame_equal(df1, df2, by_blocks=by_blocks_fixture, obj=obj_fixture)
```

However it looks the the test function isnt being used anywhere:
```
$ grep -rnw . -e "test_frame_equal_unicode"
./pandas/tests/util/test_assert_frame_equal.py:217:def test_frame_equal_unicode(df1, df2, msg, by_blocks_fixture, obj_fixture):
```

Wasn't sure if I should leave as is? or assume person calling the function will format `msg` properly before passing to the test function.

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

Comments:
Thanks @ssikdar1
BLD: More lightweight mypy pre-commit hook
State: closed

Fixes #30811 
This now will only detect typing problems in the files you have edited. This may still lead to typing problems detected in CI but is hopefully a compromise between speed and early detection of issues before they hit public CI. One line changes in a single file still stay for me in the <1s mark.

cc @gfyoung @jorisvandenbossche 

Labels:
name: Build
description: Library building on various platforms

Comments:
@pandas-dev/pandas-core : Any further thoughts on this?  I think we are ready to merge.
Given that there are no more comments, going to merge this (the 20s of time to commit is getting annoying ..)
[DOC] add example of rolling with win_type gaussian
State: closed

Admittedly this is not the first issue I address, but this one's been open for several months now and so I figured I'd take it

- [x] closes #26462
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Docs

Comments:
Thanks @MarcoGorelli
COMPAT: bump minimum version to pyarrow 0.13
State: closed

xref discussion in https://github.com/pandas-dev/pandas/pull/28371

If in the future we want to always try to import pyarrow, having pyarrow 0.13 (instead of 0.12) as the minimum required version will make this easier.

Labels:
name: Dependencies
description: Required and optional dependencies

Comments:
lgtm merge on green.
BLD: Remove mypy from pre-commit as long its always a full run
State: closed

Without supporting an incremental mode, the runtime overhead (reportably at 20s)
is unacceptable for efficient development. We can reactivate once a stable,
 incremental mode works for `pandas`.

Labels:
name: Build
description: Library building on various platforms
name: Clean

Comments:
For my understanding: what are the blockers for incremental mode for pandas? And isn't that actually what mypy does by default: https://mypy.readthedocs.io/en/latest/command_line.html#incremental-mode ?
I think we may just want to comment out for now instead of deleting entirely, so that way it is easy to restore down the road.
> For my understanding: what are the blockers for incremental mode for pandas? And isn't that actually what mypy does by default: https://mypy.readthedocs.io/en/latest/command_line.html#incremental-mode ?

Maybe this is only working on some systems as your reported numbers seem to be no different between cached and non-cached runs. Can you try to run with the [SQLite cache](https://mypy.readthedocs.io/en/latest/command_line.html#cmdoption-mypy-sqlite-cache) instead ?
To ensure we are talking about the same thing: the 0.6 you mentioned in the other PR for a cached run, is that for a fully identical run? Or after a tiny change? 

Because what I tried was: run mypy (takes ca 20s), do a tiny non-significant 1-line change, run mypy again (still taking 20s). If I really run mypy again without any change (directly with `mypy` then, not through the commit hook, as that will be skipped if there is no change), then I also get a fast run in the order of 1s). 
> To ensure we are talking about the same thing: the 0.6 you mentioned in the other PR for a cached run, is that for a fully identical run? Or after a tiny change?
> 
> Because what I tried was: run mypy (takes ca 20s), do a tiny non-significant 1-line change, run mypy again (still taking 20s). If I really run mypy again without any change (directly with `mypy` then, not through the commit hook, as that will be skipped if there is no change), then I also get a fast run in the order of 1s).

Ah, that explains it. Yeah, we should also have fast one line changes, the 0.6s were without a change.
Ah, OK, that explains ;) 
Now, I did the timing with a tiny change, as in practice when iterating on a branch, you always commit *something*, so the time it takes for mypy without change doesn't really seem relevant for the pre-commit hook (eg also when black changed something in the pre-commit hook, and you directly re-commmit, mypy again takes a long time, although black already changed it before mypy was run the first time ..)
What's the plan between #30814 and this one? If we merge #30814, then this isn't needed?
> What's the plan between #30814 and this one? If we merge #30814, then this isn't needed?

This PR here was only a immediate mitigation as I wanted to provide an option to work with less pain again. Given that we have #30814, this can be closed after merge.
Changed the milestone to `No Action` so that it is clear we are not intending to merge this one.
REF: use shareable code for DTI/TDI.insert
State: closed

xref #30757 should go in before this because it contains the tests.  After this, we'll be able to de-duplicate the two methods.

Labels:
name: Clean

Comments:
Hello @jbrockmendel! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:











There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-01-08 18:34:47 UTC
Will need an API-breaking whatsnew for changing the type of the exception.

Though to confirm: TypeErrors are typically raised when the argument has an incorrect type. In this case, the type looks correct, since it's a `Timestamp`. It's a tz-naive timestamp though, so I wouldn't expect a ValueError.


thanks @jbrockmendel
Multi Phase JSON Initialization
State: closed

Feature in Python 3.5 that should simplify instantiation of the JSON module and make it more "pythonic"

https://docs.python.org/3/c-api/module.html?highlight=multi%20phase#multi-phase-initialization
https://www.python.org/dev/peps/pep-0489/

Also removed a version string from within the extension, as I don't see where that is useful


Labels:
name: IO JSON
description: read_json, to_json, json_normalize
name: Clean

Comments:
sure
CI: Fix spelling in requirements-dev generator script
State: closed

Fixes the spelling of _dependency_ in `script/generate_pip_deps_from_conda.py`.

This script creates `requirements-dev.txt` which contains the spelling mistake, so is also fixed (using the updated script).

Labels:
name: CI
description: Continuous Integration
name: Dependencies
description: Required and optional dependencies

Comments:
Thanks for the fix @mastersplinter