BrandonFors/pandas-github-issues huggingface dataset:
CLN: reshape
State: closed

I think some of this can be made easier to reason about (like with making FrameApply less stateful), as a preliminary this is just some mild annotations etc

Labels:
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Clean
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
thanks, if you can create an issue for that deprecation
CLN: groupby
State: closed

splitting this off of branches that do non-CLN stuff

Labels:
name: Groupby
name: Clean
name: Typing
description: type annotations, mypy/pyright type checking

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 2019-11-17 15:40:56 UTC
Closing in favor of #29124
CLN: Make non-empty **kwargs in Index.__new__ fail instead of silently dropped
State: closed

In #29624 I made change so random kwargs weren't passed to ``Index._simple_new``. I'v found out after merging that the selected solution dropped non-empty kwargs, which isn't great either.

This version ensures that passing non-accepted kwargs raises, so ``pd.Index(['a'], foo="foo")`` raises a TypeError. Instead of raising a TypeError, should we deprecate adding non-accepted kwargs?


Labels:
name: Clean

Comments:
lgtm. merge on green.
Update nullable integer docs with None instead of np.nan
State: closed

Update nullable integer docs with None instead of np.nan

- [X] closes #29599 
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Docs

Comments:
This seems fine.

I'm not sure whether avoiding the original problem (a trip through a float dtype ndarray) is worth fixing or not, when the user provides `np.nan` (which is a float).
thanks @lucassa3 
@jreback no problem. Glad I could help ^^
Triage guide
State: closed

Discussed on the dev call yesterday.

cc @pandas-dev/pandas-core. Any sections I'm missing? Any statements you disagree with?

xref https://github.com/pandas-dev/pandas-governance/issues/11 (writing down a policy for triaging once we have one).

Labels:
name: Docs

Comments:
Awesome - great to have this info documented! @TomAugspurger 
@alimcmaster1 a couple questions, since you're our only triager so far:

1. Can you close PRs, in additional to issues?
2. Can you push to PRs that aren't your own? I see a "Add more commits by pushing to ..." just above the CI checks.
> @alimcmaster1 a couple questions, since you're our only triager so far:
> 
> 1. Can you close PRs, in additional to issues?
> 2. Can you push to PRs that aren't your own? I see a "Add more commits by pushing to ..." just above the CI checks.

1) Yes triage access allows you to close PRs
2) No I cannot push to PRs that aren't my own.

If helpful the full permissions matrix is here:
https://help.github.com/en/github/setting-up-and-managing-organizations-and-teams/repository-permission-levels-for-an-organization

Thanks!
Thanks. Hopefully addressed everything in 5772227e187ff0403cb432f85884b955a3495ae9

thanks @TomAugspurger very nice!
One thing I have noticed:

The triage team gives me access to cancel/restart build jobs on Travis but not on Azure. 

I can add this info if we think its worth it. (But I know we are hoping to migrate off travis soon)

Anyway just thought I would highlight
Fixed tokenizer build warnings
State: closed

Fixes the following warnings:

```sh
pandas/_libs/parsers.c:5873:52: warning: incompatible pointer types assigning to 'double (*)(const char *, char **, char, char, char, int)' from 'double (const char *,
      char **, char, char, char, int, int *, int *)' [-Wincompatible-pointer-types]
    __pyx_v_self->parser->double_converter_withgil = round_trip;
                                                   ^ ~~~~~~~~~~
pandas/_libs/parsers.c:5911:50: warning: incompatible pointer types assigning to 'double (*)(const char *, char **, char, char, char, int)' from 'double (const char *,
      char **, char, char, char, int, int *, int *)' [-Wincompatible-pointer-types]
    __pyx_v_self->parser->double_converter_nogil = precise_xstrtod;
                                                 ^ ~~~~~~~~~~~~~~~
pandas/_libs/parsers.c:5940:50: warning: incompatible pointer types assigning to 'double (*)(const char *, char **, char, char, char, int)' from 'double (const char *,
      char **, char, char, char, int, int *, int *)' [-Wincompatible-pointer-types]
    __pyx_v_self->parser->double_converter_nogil = xstrtod;
                                                 ^ ~~~~~~~
pandas/_libs/parsers.c:23829:93: warning: incompatible function pointer types passing 'double (*)(const char *, char **, char, char, char, int)' to parameter of type
      '__pyx_t_5numpy_float64_t (*)(const char *, char **, char, char, char, int, int *, int *)' (aka 'double (*)(const char *, char **, char, char, char, int, int *, int
      *)') [-Wincompatible-function-pointer-types]
          __pyx_v_error = __pyx_f_6pandas_5_libs_7parsers__try_double_nogil(__pyx_v_parser, __pyx_v_parser->double_converter_nogil, __pyx_v_col, __pyx_v_line_start...
                                                                                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pandas/_libs/parsers.c:2716:115: note: passing argument to parameter here
static CYTHON_INLINE int __pyx_f_6pandas_5_libs_7parsers__try_double_nogil(parser_t *, __pyx_t_5numpy_float64_t (*)(char const *, char **, char, char, char, int, i...
                                                                                                                  ^
4 warnings generated.
```

Labels:
name: IO CSV
description: read_csv, to_csv
name: Internals
description: Related to non-user accessible pandas implementation

Comments:
thanks @WillAyd
CLN: reachable cases in Reducer
State: closed

compute_reduction has optional `labels` and `dummy` arguments.  In practice, either both are passed or neither is passed.  By enforcing this, we can simplify the code a bit.

Labels:
name: Internals
description: Related to non-user accessible pandas implementation
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 2019-11-14 15:37:22 UTC
sure, can you merge master.
thanks
DEPR: enforce nested-renaming deprecation
State: closed

Looks like this deprecation was introduced in #14668 (April 2017), so should be OK to remove by now.

Suggestions for better error messages welcome.

Not passing `_level` around is pretty nice

Labels:
name: Deprecate
description: Functionality to remove in pandas
name: Clean

Comments:
@jorisvandenbossche i could use some help figuring out what the intended post-deprecation-enforcement behavior is.  From the test failures, I'm guessing some of these cases should not raise?
@jbrockmendel can you give an example of a case you are unsure about?
> can you give an example of a case you are unsure about?

Since the test failures have been fixed, the remaining question is if there are better error messages we should give
> can you add the whatsnew for this in removals & update the deprecation removal issue.

added (but not checked off) to #13777
thanks!
STYLE: Specify target-version for black
State: closed

The `target-version` option explicitly tells `black` that it can use Python features from the supplied versions.  The default is per-file detection by looking for the presence specific syntax, e.g. f-strings would indicate py36.

Currently `black` does not detect this for us since we just bumped to py36, so detection will not occur until f-strings are added, which would in turn necessitate these changes as part of the f-string PRs, making them a bit more convoluted than needed.  This resolves that issue by explicitly telling `black` our supported versions and enforcing those changes for the entire codebase in this PR.

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

Comments:
can u update the template and pre-commit hooks  as well?

or do they read the pyproject.toml?
I tested it locally and it looks like it reads the `pyproject.toml` by default, like how `flake8` reads from `setup.cfg` by default with pre-commit, etc.
Yes, when we started using black, we didn't yet have the pyproject.toml, so needed to put the config flags into the CI call itself. But now we have that, we should indeed move the config there.
lgtm. can you merge master (conflict); merge on green.
thanks @jschendel !
CLN:Typing in pandas/core/dtypes/
State: closed

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


Labels:
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
lgtm.
thanks @MomIsBestFriend
CLN: Remove references to user branch (follow up)
State: closed

Follow ups from 
https://github.com/pandas-dev/pandas/pull/29513
and
https://github.com/pandas-dev/pandas/pull/25427

(needed for network tests)

- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`


Labels:
name: Clean

Comments:
LGTM
@alimcmaster1 general comment: can you give this PR a title that is somewhat clearer?
> @alimcmaster1 general comment: can you give this PR a title that is somewhat clearer?

Yes sorry - agree
thanks!

@alimcmaster1 would you mind trying to fix this build: https://travis-ci.org/pandas-dev/pandas/jobs/611742966 (its the allowed failure build); i think it just needs a new reference for an html file. I think we have an issue about this somewhere.
@alimcmaster1 thanks! Much better ;), that always helps when skimming the prs/issues
Thanks both! Yes @jreback ill take a look this eve
fixed issue of empty dataframe combine_first
State: closed

- [ ] closes #29562
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry - fixed issue of combining two empty dataframe. 


Labels:
name: Bug
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
also pls do an issue search, I *think* there might be other issues out there like this
Hello @iamrajhans! 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-20 13:15:31 UTC
@jreback I have done the changes, please review
@jreback could you please take a look
looks like some merge conflicts; can you fix up and try to get CI green?
sure will make those changes


@iamrajhans looks like still red - have time to look and get green?
Closing as I think this has gone stale, but ping if you'd like to pick back up
CLN: f-string at pandas/_libs/tslib.pyx
State: closed

Continuation of previous [PR](https://github.com/pandas-dev/pandas/pull/29554)

- [x] tests passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

ref to issue #29547 

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

Comments:
thanks !
CLN: Removing commented code at pandas/_libs/parsers.pyx
State: closed

Did as @jbrockmendel suggeted [here](https://github.com/pandas-dev/pandas/pull/29554#discussion_r344942442)

- [x] tests passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Labels:
name: IO CSV
description: read_csv, to_csv

Comments:
thanks
CLN: no longer need to catch AttributeError, IndexError
State: closed

cc @jreback @WillAyd 

As of today, we don't call `agg_series` unless `ngroups > 0`, which has the side-benefit that we don't call unless `len(self.bins) > 0`, which means we no longer risk hitting an `IndexError` in libreduction L262.

No longer needing to catch AttributeError is [mumble mumble...] I'll figure out what past-me was thinking after a good night's sleep.

Labels:
name: Clean
name: Apply
description: Apply, Aggregate, Transform, Map

Comments:
thanks
REF: check instead of catching TypeError
State: closed


Labels:
name: Groupby
name: Clean

Comments:
thanks @jbrockmendel 
@jreback if I review a PR with a comment, can you try to give me a little bit of time to respond to Brock's response?

@jbrockmendel thanks for that reference. So in that test case, you have functions that return single numbers. 
But, you can also have an aggregation that returns an array-like or list, which has a dimension, but cannot be concatted. Quick dummy example with the case from the tests:

```
In [8]: df = DataFrame( 
   ...:     {"A": [None, 2, 3], "B": [1.0, np.nan, 3.0], "C": ["foo", None, "bar"]} 
   ...: )  

In [9]: df.agg([lambda x: list(x.unique())]) 

Out[9]: 
                        A                B                 C
<lambda>  [nan, 2.0, 3.0]  [1.0, nan, 3.0]  [foo, None, bar]
```

This now fails. 
(the above example looks a bit unnatural, but in general, returning lists from an aggregation is something that certainly gets done I think, also in groupby context)

@jorisvandenbossche i gave a day here which is reasonable
if you want to block then put a hold on the review
@jreback while i agree an explicit "hold" would be helpful, I think Joris gets some extra slack because of the timezone differences.

@jorisvandenbossche have i mentioned recently how much I dislike listlike corner cases?  But the case you brought is Technically Correct, so I'd be OK if you want to revert (and add that test case)
> @jreback while i agree an explicit "hold" would be helpful, I think Joris gets some extra slack because of the timezone differences.

Indeed ;) For me, the "1 day" (15 hours since Brocks comment) was just a morning after having slept. 
Maybe just interpret a comment that is not accompanied with a github approve as a sign that I want to check back?

> But the case you brought is Technically Correct, so I'd be OK if you want to revert (and add that test case)

Do you directly think of another way to do this that you would prefer more than the try/except? Otherwise I can indeed do a revert yes.


> Do you directly think of another way to do this that you would prefer more than the try/except?

The check in this PR was the best idea I had.  Do you have any suggestions?
We could check for actual series/frame objects (since concat only allows that), but personally, I find this a perfect use case for the try except.
Added a test case here: https://github.com/pandas-dev/pandas/pull/29632
DOC: Removing duplicated ecosystem and roadmap in the docs
State: closed

The roadmap and ecosystem pages were moved to the website, but left in the docs initially. This removes the duplicated files in the docs.

Labels:
name: Docs
name: Clean

Comments:
Did we reach a conclusion on the call yesterday?

IIRC, @jorisvandenbossche preferred keeping the contributing guide in the docs and linking to the dev version from the website. I think that's my preference.

No strong preference on the roadmap.
My understanding is that it was already decided that ecosystem and roadmap were moved to the web, and it was just pending removing the doc version.

Yesterday's discussion was about the contributing page, if I didn't misuderderstand.

But let's double check that everybody is happy with this before moving forward.
@jorisvandenbossche is this ok, or is the discussion on where the ecosystem and roadmap go open?

Would be good to get rid of one of the copies before we make changes to them.
Any further comment here? If there is no agreement on this I'll just close and let someone else take care of this.
Sorry for the slow reply here. And yes, the discussion on the call was mainly about the contributor docs, not the roadmap/ecosystem.

I personally have a slight preference for keeping the roadmap together with all other development/contributing related information. But, if that decision was taken and that's the preference of most, I also don't want to reopen this (so in that case, go ahead with this), it's no strong preference (sorry, I honestly don't remember any more what decision we exactly took on this before in the initial web PR). 

The ecosystem seems fine to have on the website. In any case in the current docs it's also a bit "lost" somewhere. I actually removed it from the top navigation bar, because I didn't feel it belonged in the first-level navigation, so it's currently "orphaned". 
No decision was taken, other than the PR that added those pages to the website.

My view was that to avoid linking the dev version of the docs was important. That was the main reason why I didn't use sphinx for the website, but a separate static site generator. My plan was to have everything that we want users to see the version from master in the website.

Since this is finally not happening, I don't have a preference on where to have things.

But I thing we shouldn't keep two versions for long, when there are changes to those pages it'll start to be tricky and time consuming to find differences and keep the updated content in the page that finally stays. 

Closing this for now, but I thing would be good if you can make a decision soon.
BUG: Fix DatetimeIndex.strftime with NaT present
State: closed

- [X] closes #29578
- [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: Datetime
description: Datetime data dtype
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

Comments:
thanks @jschendel
TYP: annotate _data
State: closed

NB: this does induce a small behavior change in that `Index` will not have a `_data` attribute until `__init__/__new__`.

I found in #29561 that getting `_data` annotated is necessary before we can annotate most of `core.indexes`.

I'm also not sure what to do about MultiIndex, suggestions welcome.

Labels:
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
lgtm. @simonjayhawkins @WillAyd 
thanks @jbrockmendel
CLN: F-string formatting in pandas/tests/indexes/*.py (#29547)
State: closed

- [ ] closes #xxxx
- [x] 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'm a little unsure how to interpret the results of the checks - since code_checks.sh exits with an error at a line where isort is called, it seems like it would point to an issue with importing. However, I haven't changed any import statements, and I checked that all the pytests were consistent with before I changed anything, so I'm not sure what the issue is.
Hi @yashukla the issue is related to black opposed to isort see:

```
black, version 19.3b0
Checking black formatting
would reformat /home/vsts/work/1/s/pandas/tests/indexes/test_common.py
```
can you run `black pandas` on the changes?
Also please could you reference the issue:

https://github.com/pandas-dev/pandas/issues/29547

thanks!
@alimcmaster1 Oh, I wasn't using black correctly. Thanks so much! Correct issue referenced, formatting fixed, and code now passes `black pandas` and `git diff upstream/master -u -- "*.py" | flake8 --diff`, along with checks; anything else that needs to be done?
thanks @yashukla
CLN: Update .format(...) strings to f-expressions
State: closed

- [x] xref  #29547
- [ ] 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:
Thanks - please link the issue https://github.com/pandas-dev/pandas/issues/29547
CI failures related to black

```
All done! 💥 💔 💥
2 files would be reformatted, 865 files would be left unchanged.
Checking black formatting DONE
```

Please run `black pandas`
thanks @srinivasreddy
BUG: GH29595 fix read_json() to use utf-8 for a default encoding
State: closed

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


Labels:
name: IO JSON
description: read_json, to_json, json_normalize
name: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
thanks @ryanking13
BUG: np.datetime64 - TimedeltaArray
State: closed

- [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: Datetime
description: Datetime data dtype

Comments:
thanks. its unlikely but possible we have an open issue for this
CLN:F-string formatting
State: closed

Ref to #29547 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`


Labels:
name: Clean

Comments:
ping @jbrockmendel 
trailing whitespace
gentle desperate ping @jbrockmendel 
LGTM cc @WillAyd
thanks @MomIsBestFriend 

definitely more readable with the f-strings, keep em coming!
Revert "CI: workaround numpydev bug (#29433)"
State: closed

This reverts commit b4adb71aee21fcbda3fed32471c60817a8bd5c9f.

Closes #29432 

Labels:
name: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
Hmm a few warnings snuck in.

@jorisvandenbossche is fixing some in #29549 .

I won't have time today or tomorrow for the rest. Anyone able to push fixes to my branch?
at least some of these look like the result of #29517; ill see what i can do about it
pushed a hopeful-fix, will see how it goes
lgtm. any perf implications? (IOW now we have to catch *moar* warnings)?
> any perf implications? (IOW now we have to catch moar warnings)?

It shouldn't be any worse than the previous solution we had in place that did specific dtype checks to prevent the warnings.
I'm not sure we should be filtering the warnings like this. IIUC this may break in the future when NumPy enforces their deprecation.

I don't know enough of the context for https://github.com/pandas-dev/pandas/pull/29517/files, but we don't want the behavior to silently change for the user. If we like the change from NumPy, then we should not filter the warnings. If we don't like it (say because we need more flexible dtype comparison) then I think we need to restore some of the checks in #29517 (again, I haven't read through the changes there in detail).
Tom makes a fair point.  I'd be OK with reverting #29517 since having this build is clearly more important.
what if we filter on the specific warning, is that better?
just pushed a commit that restored the checks that were removed by #29517.  I may try to remove them again someday, but getting this build back up is a higher priority
do we need this?
"This" being the commit Brock pushed up in https://github.com/pandas-dev/pandas/pull/29553#issuecomment-554589699? I think so... That at least gets us back to where we were, and gets us testing against numpydev again.
> "This" being the commit Brock pushed up in [#29553 (comment)](https://github.com/pandas-dev/pandas/pull/29553#issuecomment-554589699)? I think so... That at least gets us back to where we were, and gets us testing against numpydev again.

right. i guess if you can rebase this and see
Merged master.
this should be good to go?
thanks @TomAugspurger and @jbrockmendel 

maybe open a followup issue to re-remove this at some point if possible?