BrandonFors/pandas-github-issues huggingface dataset:
CLN: Clean frame/test_constructors.py
State: closed

I think these classes are all imported directly so don't need to reference the pandas namespace

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

Comments:
OK as a one off, though @jorisvandenbossche I think and I generally prefer the way things are here. 

I know @jreback disagrees and prefers what you've done, but we don't have a consensus to do things like this generally
> OK as a one off, though @jorisvandenbossche I think and I generally prefer the way things are here.
> 
> I know @jreback disagrees and prefers what you've done, but we don't have a consensus to do things like this generally

I was thinking the tests might run ever so slightly faster if we refer to (for example) `Series` instead of `pd.Series` when it's already imported
what I don't want are *mixed* uses in a single file, IOW ``pd.Series`` and ``Series`` are not good. slight preference to using ``Series``,  but wouldn't wholesale change things.
> I was thinking the tests might run ever so slightly faster if we refer to (for example) `Series` instead of `pd.Series` when it's already imported

this is not a consideration at all. In testing its all about coverage and clarity, while being as concise as possible.


> OK as a one off, though @jorisvandenbossche I think and I generally prefer the way things are here.

That's correct 
(in general we should for sure try to use the public namespeace where possible in tests. Of course, `from pandas import Series` and `import pandas as pd` are both perfectly fine public ways to access Series in this case (on constract to eg `from pandas.core.series import Series`). But using `pd` for top-level objects is closer to how I (and I think most contributors) typically code, and avoids long imports)

Also agreed with @jreback that consistency within a file is more important. 

thanks @dsaxton
Avoid bare pytest.raises in dtypes/cast/test_upcast.py
State: closed

* [x]  ref #30999
 
* [x]  tests added / passed
 
* [x]  passes `black pandas`

* [x]  passes `git diff origin/master -u -- "*.py" | flake8 --diff`

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

Comments:
Thanks @Vlek
BUG: Fix file descriptor leak
State: closed

- [x] closes #31488 
- [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: IO CSV
description: read_csv, to_csv

Comments:
thanks @roberthdevries
REF: implement nanops.na_accum_func
State: closed

this is a follow-up that was requested a few months ago

Labels:
name: Refactor
description: Internal refactoring of code
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

Comments:
@TomAugspurger thoughts on whether we're going to do a 1.0.3?  if so i should hold off on refactors like this
I'm not sure if we're doing a 1.0.3. Will talk about it on the call today.

On Tue, Mar 10, 2020 at 9:46 PM Jeff Reback <notifications@github.com>
wrote:

> *@jreback* commented on this pull request.
>
> @TomAugspurger <https://github.com/TomAugspurger> thoughts on whether
> we're going to do a 1.0.3? if so i should hold off on refactors like this
>
> this is for 1.1 right?
> ------------------------------
>
> In pandas/core/nanops.py
> <https://github.com/pandas-dev/pandas/pull/32597#discussion_r390718006>:
>
> > @@ -1497,3 +1497,62 @@ def nanpercentile(
>              return result
>      else:
>          return np.percentile(values, q, axis=axis, interpolation=interpolation)
> +
> +
> +def na_accum_func(values, accum_func, skipna: bool):
>
> can you add a doc-string
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/32597?email_source=notifications&email_token=AAKAOIVVSZIWQKSDNGM73RDRG33RFA5CNFSM4LFIRB5KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCYZQZ5I#pullrequestreview-372444405>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAKAOIXADB3VJZCG723LK6DRG33RFANCNFSM4LFIRB5A>
> .
>

thanks
BUG: Don't multiply sets during construction
State: closed

- [x] closes #32582
- [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: Constructors
description: Series/DataFrame/Index/pd.array Constructors

Comments:
Hmm how is this type of construction actually useful? If anything this should probably just raise
> Hmm how is this type of construction actually useful? If anything this should probably just raise

Yeah, I could see that too. Looks like it does raise in the `Series` case so makes sense it would here too (or could treat sets as singletons, although that seems even less useful?)
If Series raises then I think that is even more motivation to raise here; this seems more likely to be a programmer error than intent
thanks @dsaxton
REG: dt64 shift with integer fill_value
State: closed

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


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

Comments:
> Do we have a policy on introducing warnings in bugfix releases?

According to https://pandas.pydata.org/docs/development/policies.html 

> We will not introduce new deprecations in patch releases.

But perhaps it's OK here since this is fixing a bug (failure to deprecate something in 1.0). I don't have a strong preference.
@jbrockmendel can you update doc-strings, otherwise lgtm.
updated+green; i think ive addressed the relevant comments
thanks, let's see if this backports
@meeseeksdev backport 1.0.x
Something went wrong ... Please have  a look at my logs.
@TomAugspurger  did this get backported successfully or do i still need to do it manually?
https://github.com/pandas-dev/pandas/pull/32647 backported it. I think the thing about the error is from the auto-backport and Jeff's manual request kicking in around the same time, so the second one errored since the backport was already created (maybe).
right i am sure not if things backport so i usuually kick the bot which i think is safe (it won’t do it twice)

but found that if it needs a manual backport it shows immediately which is good
CLN: remove unreachable code in pandas/core/groupby/generic.py::DataFrameGroupBy::_transform_general
State: closed

Is this part of the code
```
        else:
            # Handle cases like BinGrouper
            return self._concat_objects(keys, values, not_indexed_same=not_indexed_same)
```
necessary? The condition before it is
```
elif self.grouper.groupings is not None
```
. However, even in the case of `BinGrouper`, I don't believe `BinGrouper.groupings` can be `None`:
```
>>> from pandas.core.groupby.ops import BinGrouper
>>> BinGrouper([], []).groupings                                                                                                                                                                   
[Grouping(None)]
```

 and so this code is unreachable. It's not covered by the tests, at least - https://codecov.io/gh/pandas-dev/pandas/src/master/pandas/core/groupby/generic.py#L1354

If it can be removed, I've taken it out and then shifted the code above it back by one tab (because we no longer need the guard)

Labels:
name: Groupby
name: Clean

Comments:
IIRC the coverage results dont include the slow tests.  did you run those?
> IIRC the coverage results dont include the slow tests. did you run those?

Thanks, wasn't aware of that.

Don't they run as part of CI? If so, looks like they still pass
@MarcoGorelli still active? Can you address latest comments?
Sorry for the delay - yes, I'll get to this during the weekend

EDIT
----
am pretty time constrained right now, will get to this next weekend
Closing for now, as I'm busy with other things and have other PRs to respond to: I've opened this up as a good first issue: #33478 .

If nobody takes the issue, I'll come back to it
REG: Restore read_csv function for some file-likes
State: closed

Restore `read_csv` function for some file-likes
    
Restores behavior down to the fact that the Python engine cannot handle NamedTemporaryFile.
    
Closes https://github.com/pandas-dev/pandas/issues/31819
    
Credit to @sasanquaneuf for [originating idea](https://github.com/pandas-dev/pandas/issues/31819#issuecomment-584529415).

Labels:
name: Regression
description: Functionality that used to work in a prior pandas version
name: IO CSV
description: read_csv, to_csv

Comments:
Alrighty, everything is finally green!  If people can take a look at and give feedback on this, that would be great so we can get this merged before tomorrow.
TST: Add tests for duplicated and drop_duplicates
State: closed

- [x] refers to  #15752
- [x] tests added / passed
- [x] tests duplicated and drop_duplicates for period, categorical, datetimes, timedelta 

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

Comments:
Hello @mproszewska! 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-04-03 23:00:42 UTC
can you merge master 
thanks @mproszewska very nice
PERF: copy cached attributes on index shallow_copy
State: closed

- [x] closes #28584
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

The performance of the example in #28584 is:

```python
>>> idx = pd.Index(np.arange(100_000))
>>> %timeit idx.get_loc(99_999)
1.17 µs ± 80.6 ns per loop  # master and this PR
>>> %timeit idx._shallow_copy().get_loc(99_999)
3.57 ms ± 286 ns per loop  # master
3.67 µs ± 286 ns per loop  # this PR
```

The issue is still on the extension indexes, e.g. ``CategoricalIndex._shallow_copy``. I'd like to take them afterwards.

Labels:
name: Performance
description: Memory or execution speed performance
name: Index
description: Related to the Index class or subclasses

Comments:
LGTM pending green
Should engine be shared?  If so, maybe access it to ensure it is in _cache before coping _cache?
If the ``_cache`` has been populated before the call to ``_shallow_copy``, we always copy. It it hasn't, it's a question of evaluating trade-offs:
1. If we access ``_engine`` before copying, we risk calculating unneccesarily
2. If we do not access it before copying, we risk having to do the same calculation several times.

My hunch is that it's mostly not that important, because either the cache has already been populated, or the original index will not be used again (e.g. in a pipe, often).

But in some given cases we'd prefer either 1 or 2, depending on context, but we'd have to choose. I prefer 2., because in that case ``_shallo_copy`` will never halt to create new ``_engine`` attributes, i.e. performance will be more predictable.

Makes sense, thanks for explaining your thought process.
thanks @topper-123 

if you think we need additional asvs's happy to have them (e.g. is_monotonic, is_unique), MI indexing, and .get_loc would all benefit; we might have enough, just checking in about this.
BUG: Fix segfault in csv tokenizer
State: closed

- [x] closes #28071
- [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: IO CSV
description: read_csv, to_csv

Comments:
Hello @roberthdevries! 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-03-15 07:19:35 UTC
@roberthdevries pls merge master; ping on green.
ping @jreback everything is green
thanks @roberthdevries
TST: Fix bare pytest raises in generic/test_frame.py
State: closed

- [x] refers https://github.com/pandas-dev/pandas/issues/30999
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Adds match arguments to pytest.raises calls in pandas/tests/generic/test_frame.py


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

Comments:
LGTM
Thanks @andresmcneill
DOC: Add missing question mark icon
State: closed

- [x] closes #32469
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

I added the question mark to the `_static` folder. When rebuilding the documentation pages locally using the latest version of the `pydata-bootstrap-sphinx-theme`, the `div.highlight` seems to be styled correctly (`background: white`), removing the _strange green around the code block_.


Labels:
name: Docs

Comments:
So to understand, you forgot to include this in the initial PR, and it was unrelated to the changes in the theme?
With respect to the `svg`, I do think this must have been the case (file was ignored by default, so I had to `add -f`). The background of the code section seems to be a theme-related adjustment which is no longer apparent using the latest version of the theme.
Ensure valid Block mutation in SeriesBinGrouper.
State: closed

Closes https://github.com/pandas-dev/pandas/issues/31802

~This "fixes" #31802 by expanding the number of cases where we swallow an
exception in libreduction. Currently, we're creating an invalid Series
in SeriesBinGrouper where the `.mgr_locs` doesn't match the values. See
https://github.com/pandas-dev/pandas/issues/31802#issuecomment-595954511
for more.~

~For now, we simply catch more cases that fall back to Python. I've gone
with a minimal change which addresses only issues hitting this exact
exception. We might want to go broader, but that's not clear.~


cc @jbrockmendel & @WillAyd 

Labels:
name: Groupby
name: Regression
description: Functionality that used to work in a prior pandas version

Comments:
I could go either way. My long-term hope is to get away from using exceptions as control flow like this. I'm not sure whether `except Exception` gets us closer or further from that goal (probably further) but it does fix regressions.
Per comment in #31082, would it be viable to check in libreduction before setting incorrectly-shaped block.values?
I don't have a firm grip on when this occurs, but it seems like it should be any time you have a change in the group size. But I might be incorrect.
https://github.com/pandas-dev/pandas/pull/32561/commits/ad746ba68752dbf0d398dbd7572efe702edd2657 changed things to also mutate the Block.mgr_locs in addition to the values, so the earlier discussion on what to catch is moot (as far as this PR is concerned).
I think some unrelated (already merged in master) edits have snuck in
Fixed the git snafu I think.
Has anyone seen the 32bit failure elsewhere?

```
=================================== FAILURES ===================================
___________________ TestDataFrameAnalytics.test_stat_op_calc ___________________
[gw0] linux -- Python 3.6.7 /home/vsts/miniconda3/envs/pandas-dev/bin/python

    
        def kurt(x):
            from scipy.stats import kurtosis  # noqa:F811
    
            if len(x) < 4:
                return np.nan
            return kurtosis(x, bias=False)
    
        assert_stat_op_calc(
            "nunique",
            nunique,
            float_frame_with_na,
            has_skipna=False,
            check_dtype=False,
            check_dates=True,
        )
    
        # mixed types (with upcasting happening)
        assert_stat_op_calc(
>           "sum", np.sum, mixed_float_frame.astype("float32"), check_dtype=False,
        )

pandas/tests/frame/test_analytics.py:321: 

```

I guess https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=30468&view=logs&j=3a03f79d-0b41-5610-1aa4-b4a014d0bc70&t=4d05ed0e-1ed3-5bff-dd63-1e957f2766a9&l=74 had it on the npdev build. Is that a flaky test?
> Has anyone seen the 32bit failure elsewhere?

Yes.  Best guess is #32571 caused this, am troubleshooting now
TST: Add extra test for pandas.to_numeric() for issue #32394
State: closed

- [x] closes #32394
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry (covered by PR #32541)


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations

Comments:
@jreback 
@roberthdevries if you can add the OP test as well here. thanks.
OP test added
thanks @roberthdevries
Backport PR #32242 on branch 1.0.x (BUG: Fixed bug, where pandas._libs.lib.maybe_convert_objects function improperly handled arrays with bools and NaNs)
State: closed

Backport PR #32242: BUG: Fixed bug, where pandas._libs.lib.maybe_convert_objects function improperly handled arrays with bools and NaNs

Labels:

Comments:
I think the azure failure is from https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=30283&view=logs&j=5291ec4e-9543-503b-43ce-895cf0e06ba3&t=ba0a2b35-e1f8-54df-fd66-2b9fde6f4b1f&l=38. statsmodel's DNS service was having issues earlier today, perhaps that's related.
The failures at https://travis-ci.org/pandas-dev/pandas/jobs/660210730?utm_medium=notification&utm_source=github_status are more concerning.
Hmm what's the point of that repo? Looks like the travis config needs updating (still using Python 3.5?) maybe a conflict in requirements is responsible
> The failures at https://travis-ci.org/pandas-dev/pandas/jobs/660210730?utm_medium=notification&utm_source=github_status are more concerning.

That points to a green build ?
Sorry, I restarted it :)
Going to merge this, since all the CI failures should have resolved themselves.
DOC: Fix EX01 in pandas.DataFrame.idxmax
State: closed

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

Labels:
name: Docs

Comments:
Thanks for the input, @MarcoGorelli!
Thanks @farhanreynaldo
DOC: Added docstring for Series.name and corrected docstring guide
State: closed

The main contribution is to add a docstring with examples for the "name" property of the Series object.  Also corrected some typos and grammatical points in the "pandas docstring guide".

The type hint gives the type as "Label" and I could not find any other reference to a custom type defined in pandas._typing which was explicitly mentioned in the docs (instead they specify "str" or "int" or somesuch), so I chose "Label (int, str or other hashable object)".
 
Further I chose "whenever displaying the Series in the interpreter" as clearer alternative to "when invoking the __repr__ method" or a similar precise statement.

Labels:
name: Docs

Comments:
Thanks @datapythonista , I considered making two separate PRs - in future I'll follow your advice!
Great thanks @dan1261
BUG: Add extra check for failing UTF-8 conversion
State: closed

- [x] closes #23809
- [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: IO Excel
description: read_excel, to_excel
name: Segfault
description: Non-Recoverable Error

Comments:
Can you check any impact to the CSV reader as well? Based off of where this fix is made I would imagine this impacts that as well as excel, so maybe should add a test / document fix for both
lgtm. defer to @WillAyd for testing.
Great thanks @roberthdevries
BUG: Dataframe.groupby aggregations with categorical columns lead to incorrect results.
State: closed

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


Labels:
name: Groupby
name: Categorical
description: Categorical Data Type

Comments:
@WillAyd @jreback thanks for your reviews.

The solution I've pushed can either be:
```
if any(is_categorical(ping.grouper) for ping in self.grouper.groupings):
    result = result.reindex(self.grouper.result_index)
```
(as this is only necessary if there's a categorical grouping), or, if the `if` statement adds unnecessary complexity, just
```
result = result.reindex(self.grouper.result_index)
```
(which works all the time but in some cases isn't necessary), as I've currently done.
This is currently wrong: it works for

```
>>> df = pd.DataFrame({"A": pd.Categorical(['a', 'b', 'a'], categories=['a', 'b', 'c']), "B": [1, 2, 3], "C": ['a', 'b', 'a']})
>>> df.groupby(['A', 'C']).transform('sum')
```

but not for

```
>>> df = pd.DataFrame({"A": pd.Categorical(['a', 'b', 'a'], categories=['a', 'b', 'c']), "B": [1, 2, 3], "C": ['a', 'b', 'a']})
>>> df.groupby(['A', 'C'])['B'].transform('sum')
```

will update when this is addressed

UPDATE
------
I think it's fine now
ok looks good. can you run the groupby benchmarks to make sure this doesn't regress anything. ping on green (and post results)
@jreback here are the results of
```bash
$ asv continuous -f 1.1 upstream/master HEAD -b ^groupby
```
:
```
       before           after         ratio
     [dede2c7a]       [d6043ecb]
     <32494^2>        <32494>   
+         185±7μs          216±8μs     1.17  groupby.GroupByMethods.time_dtype_as_field('object', 'count', 'direct')
-        826±60μs          746±6μs     0.90  groupby.GroupByMethods.time_dtype_as_group('float', 'cumprod', 'transformation')
-        208±20μs          186±1μs     0.89  groupby.GroupByMethods.time_dtype_as_group('float', 'cumcount', 'direct')
-      1.02±0.1ms         862±10μs     0.84  groupby.GroupByMethods.time_dtype_as_group('int', 'rank', 'transformation')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE DECREASED.
```
@jreback 

I just merged master and this time got
```
BENCHMARKS NOT SIGNIFICANTLY CHANGED.
```
running the same command
thanks @MarcoGorelli
BUG: pd.ExcelFile closes stream on destruction
State: closed

- [x] closes #31467
- [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: IO Excel
description: read_excel, to_excel

Comments:
You've got a conflict, if you can merge master and fix it please.
Rebased to master
Thanks @roberthdevries.
Fix failure to convert string "uint64" to NaN
State: closed

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


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

Comments:
Great thanks @roberthdevries - keep the PRs coming!
@WillAyd this has a number of issues in testing

pls revert

the OP is not tested at all 
right just need another PR; @roberthdevries can u push a test for to_numeric

@WillAyd remember to set the version in the PR and issue before merging
@jreback I will add another PR with a test for to_numeric referencing the original issue