BrandonFors/pandas-github-issues huggingface dataset:
TYP: add Shape alias to pandas._typing
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

xref https://github.com/pandas-dev/pandas/pull/37024#discussion_r503488032

cc @simonjayhawkins 

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

Comments:
@jbrockmendel ok ?
lgtm. can you merge master and ping on green.
CI green
thanks @arw2019
[REDO] CLN: core/dtypes/cast.py::maybe_downcast_to_dtype 
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

xref https://github.com/pandas-dev/pandas/pull/37050#issuecomment-708487786

The combination of #37050 and #37024 generated a `mypy` complaint. This PR is resubmission of #37050 with the correct type hint.

cc @simonjayhawkins

Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
thanks @arw2019
Add test_masking_duplicate_columns
State: closed

- [x] closes #31954
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

I don't know what an whatsnew entry is...

This PR adds a test in order to solve an issue

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves

Comments:
Somehow I missed that, it's now fixed, thanks @dsaxton 
Hello @GabrielSimonetto! 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-10-18 18:14:20 UTC
@GabrielSimonetto can you merge master and update to comments
Closing in favor of #38354
REF: IntervalArray comparisons
State: closed

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


Labels:
name: Refactor
description: Internal refactoring of code
name: Interval
description: Interval data type

Comments:
@jorisvandenbossche this is unrelated to the backing-data.  In the next commit I'll change the implementation to use left/right so as to be agnostic between the new/old backing formats.
ok i am fine with this. @jorisvandenbossche any comments.
@jorisvandenbossche i think ive addressed your comments, can you double check when convenient
Thanks for the ping, will take a look tomorrow!
ERR: Removing quotation marks in error message
State: closed

Follow-up to https://github.com/pandas-dev/pandas/pull/36852

When reviewing, I got confused and thought `iteration` was somehow an argument to `read_csv` (like `nrows`).  Since it is now, the quotation marks don't make as much sense to have.

Doing this as a follow-up since the original PR was a pure refactoring.

Labels:
name: Error Reporting
description: Incorrect or improved errors from pandas
name: IO CSV
description: read_csv, to_csv

Comments:
thanks @gfyoung
PERF: regression in DataFrame reduction ops performance #37081
State: closed

- [x] closes #37081 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Made the change proposed by @jorisvandenbossche in https://github.com/pandas-dev/pandas/pull/35881#discussion_r504035006 

Did a very quick comparison : 

With `self.dtypes` (old version) : 

```
In [8]: values = np.random.randn(100000, 4)   
   ...: df = pd.DataFrame(values).astype("int") 
   ...: %timeit df.sum() 
714 µs ± 21 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
```

With `self._iter_column_arrays()` (new version) : 

```
In [4]: values = np.random.randn(100000, 4)   
   ...: df = pd.DataFrame(values).astype("int") 
   ...: %timeit df.sum() 
477 µs ± 8.7 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
```

Labels:
name: Performance
description: Memory or execution speed performance
name: Regression
description: Functionality that used to work in a prior pandas version

Comments:
> do we have an asv that covers this case, if not can you add one?
> 
> this doesn't need a note as its on master.

I believe we do : 

https://pandas.pydata.org/speed/pandas/#stat_ops.FrameOps.time_op?p-op='sum'&p-dtype='int'
Indeed. Jeff, see the issue, it was actually catched thanks to our asv suite
Should i do something about the : 2 failed azure pipelines ?
Looks like they're failing on master too and the `./ci/code_checks.sh` localy return no error.


thanks @ukarroum
CI: check for numpy.random-related imports
State: closed

~~- [x] closes #37053 (keep this issue open until both #37492 and #37117 are completed)~~
- [x] tests added / passed ~~(once #37492is merged)~~

# update 2020-10-31
now use a pre-commit check instead of the previous code_check

*** 
~~code_check.sh will be used to locate where numpy.random-related imports are.~~ See the separate script to do the whole clean-up in https://github.com/pandas-dev/pandas/pull/37103#issue-502582592.

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

Comments:
@onshek this would either need to have selective enforcement temporarily or fix the issues first.
> @onshek this would either need to have selective enforcement temporarily or fix the issues first.

done, please make a review @jreback thanks!
@onshek you'll want to ping here when #37492 is merged (that one has to go first)
thanks @onshek
Revert "CLN: core/dtypes/cast.py::maybe_downcast_to_dtype"
State: closed

Reverts pandas-dev/pandas#37050

Labels:
name: CI
description: Continuous Integration

Comments:
opened to get mypy to green. other options available xref https://github.com/pandas-dev/pandas/pull/37050#issuecomment-708487786

cc @arw2019 
failure unrelated

```
=========================== short test summary info ============================
FAILED pandas/tests/io/test_html.py::TestReadHtml::test_banklist_url_positional_match[bs4]
= 1 failed, 88193 passed, 1148 skipped, 1446 xfailed, 74 warnings in 1718.54s (0:28:38) =
```
TST: add message matches to pytest.raises in test_duplicate_labels.py GH30999
State: closed

Reference https://github.com/pandas-dev/pandas/issues/30999

- [x] tests added / passed
- [x] passes `black pandas`
- [ ]  passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Hello! This is my first PR here. I followed the steps in the contributing guidelines [here](https://github.com/pandas-dev/pandas/blob/master/doc/source/development/contributing.rst#contributing-your-changes-to-pandas), but I didn't see anything about `git diff upstream/master -u -- "*.py" | flake8 --diff`. 

When I run the command locally, with `origin/master`, it returns no output. I get fatal with `upstream/master`. Is that expected, and if it isn't, could anyone advise on how to resolve the issue please? 

Thank you!

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Code Style
description: Code style, linting, code_checks

Comments:
Thanks both for the quick reviews. :) 

@arw2019 thanks for the specific link too, it's really helpful! The amount of information is a bit overwhelming sometimes.
thanks @theMogget very nice!
BLD: extract GH Actions check function to avoid duplication in code_checks.sh
State: closed

This PR extracts a function `if_gh_actions` in `ci/code_checks.sh` to avoid command/arguments duplication. Originally part of #36386 (cherry-picked from 8611fe68); extracted to make the changes more modular (see the cross-linked comment). 

- [N/A] closes #xxxx (xref https://github.com/pandas-dev/pandas/pull/36386#issuecomment-707115169)
- [N.A] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [N/A] whatsnew entry


Labels:
name: CI
description: Continuous Integration

Comments:
looks fine, cc @MarckK @datapythonista if any comments.

@plammens if you can merge master so we can get a green build
> @plammens if you can merge master so we can get a green build

Done; rebased onto latest master.
I personally find this more complicated than needed. I agree having the if and duplicating the call should be simplified. But I think implementing a function makes things too complex.

What I would do is instead of:
```python
    if [[ "$GITHUB_ACTIONS" == "true" ]]; then
        $BASE_DIR/scripts/validate_unwanted_patterns.py --validation-type="strings_to_concatenate" --format="##[error]{source_path}:{line_number}:{msg}" .
    else
        $BASE_DIR/scripts/validate_unwanted_patterns.py --validation-type="strings_to_concatenate" .
    fi
```
Simply have:
```python
$BASE_DIR/scripts/validate_unwanted_patterns.py --validation-type="strings_to_concatenate" --format=$FORMAT
```
Then, there are several $FORMAT that will be the same, and there is already an `if` at the beginning of the file where the format for flake8 is already defined. Having an extra format variable defined there looks simpler than wrapping all calls with a bash function, xargs...
> Simply have:
> 
> ```python
> $BASE_DIR/scripts/validate_unwanted_patterns.py --validation-type="strings_to_concatenate" --format=$FORMAT
> ```
> 
> Then, there are several $FORMAT that will be the same, and there is already an `if` at the beginning of the file where the format for flake8 is already defined. Having an extra format variable defined there looks simpler than wrapping all calls with a bash function, xargs...

Yep this actually makes more sense. I think what happened is that I cherry picked this change, without giving it much thought, from #36386, where I think a function does make sense since there's already some `xargs` going on; but in isolation this would be a better solution.

On the other hand, if we consider the possibility that in the future other commands might depend on whether they're running on GH-actions (and in a less trivial way than just changing the format), it might be worthwhile to add the function.
@jreback @datapythonista Any more thoughts on this? Should I do what @datapythonista suggested (just extracting a `FORMAT` variable) in a new PR?
most of this is done in precommit anyhow

is this PR still needed?
> is this PR still needed?

Just checked the code on master and it looks like this is no longer needed. Closing this.
PERF: Faster comparisons of indexes when compared to self
State: closed

Examples:

```python
>>> rng = pd.RangeIndex(100_000)
>>> %timeit rng == rng
429 µs ± 96.9 µs per loop  # master
14.1 µs ± 93.8 ns per loop  # this PR
>>> idx = rng.astype(int)
>>> %timeit idx == idx
347 µs ± 1.42 µs per loop  #master
14.1 µs ± 91.4 ns per loop  # this PR
>>> idx = rng.astype(object)
>>> %timeit idx == idx
5.97 ms ± 12.7 µs per loop  # master
3.72 ms ± 28.4 µs per loop  # this PR
```


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

Comments:
that works for int dtype, but what about for dtypes that have NAs?
Hmm, that’s a good point. I’ll see if anything can be done about that.
you could return `~isna()` for equal and `isna()` for not equal (if that is still faster)?
> you could return ~isna() for equal and isna() for not equal (if that is still faster)?

even then you'd need to watch out for pd.NA

I think just check for _can_hold_na.

Also for RangeIndex could compare the range objects instead of (i think) casting to ndarray
I've made an update. I can't find the tests for index comparisons, they clearly need some tests for comparisons with indexes that contain nans. My first proposal obviously shouldn't have passed.

Anyonw know where they're located?
> Anyonw know where they're located?

tests.arithmetic
would this let us remove NumericIndex._cmp_method?
@topper-123 can you merge master
@topper-123 still worth it here? can you merge master
This has already been fixed, e.g. the last example:

```python
>>> idx = rng.astype(object)
>>> %timeit idx == idx
5.97 ms ± 12.7 µs per loop  # when this PR was opened
53.9 µs ± 9.8 µs per loop  # master
```

So I'll just close this one.
TYP: fix typing errors for mypy==0.790, bump mypy version
State: closed

- [x] closes  #37104
- [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:
cc @simonjayhawkins 
Hello @fangchenli! 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 2021-01-10 03:12:54 UTC
looks ok to me, merge master just to confirm.
Thanks @fangchenli 
> merge master just to confirm.

np-dev failures seen on other PRs/master
CLN: remove unnecessary _validate_foo methods
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

since DatetimeLike and Categorical no longer special-case these, we can inline them.

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

Comments:
i guess the windows failures are spurious
VIS: Accept xlabel and ylabel for scatter and hexbin plots
State: closed

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


Labels:
name: Enhancement
name: Visualization
description: plotting

Comments:
I'm a bit puzzled by the type annotations on these params (from #34827)

https://github.com/pandas-dev/pandas/blob/9cb372376fc78aa66e2559de919592007b74cfaa/pandas/plotting/_matplotlib/core.py#L105-L106

They're `Optional[Label]` but `Label = Optional[Hashable]`. I'm all for making the Optional-ness of the parameters explicit but I'm not sure if the nesting has any side-effects, or why `Label` is `Optional` in the first place.
> I'm not sure if the nesting has any side-effects 

emm, any comments on the "potential" side-effect it might have? @simonjayhawkins  I added to make it more explicit, probably also okay if only having `xlabel: Label = None`?
lgtm. @charlesdong1991 merge when updated / green.
> emm, any comments on the "potential" side-effect it might have? @simonjayhawkins I added to make it more explicit, probably also okay if only having `xlabel: Label = None`?

not an issue. adding Optional is fine. Where the problem occurs is where we need to distinguish between user passing None as a valid label, I don't think that's relevant here.

```
from pandas._typing import Label
from typing import Optional

a: Optional[Label]
reveal_type(a)  # Revealed type is 'Union[typing.Hashable, None]'
```

Not sure why that check failed, it's way outside this PR
hmm, this seems to be related to https://github.com/pandas-dev/pandas/pull/37024 

will let you know once it's fixed so that you could rebase, sorry @treuherz 
> hmm, this seems to be related to #37024
> 
> will let you know once it's fixed so that you could rebase, sorry @treuherz

cc @arw2019 from changes in #37050
~OK @simonjayhawkins shall i revert that PR to get CI green for now?~

ah, just saw you have done that, sorry for the noise
very nice! congrats on your first PR on pandas! @treuherz
Update code_checks.sh to check for instances of os.remove
State: closed

I have included a check for instances of os.remove in the tests folder. This is in line with issue #37095 whereby ensure_clean should be used rather than os.remove.

 A few outstanding questions remain:

- Should this be expanded to search for cases of os.remove elsewhere?

- ~~Cases of `os.remove ` still exist in common.py, test_writers.py, and test_store.py. For some reason, the case in test_store.py is not picked up by this check? Any idea why?~~ _(solved by passing arguments separately rather than as a list)_

- ~~Advice on how to implement this as a pre-commit hook also welcome.~~ _(to be implemented in separate PR)_



- [x] closes #37095 
- [ ] 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:
`--exclude` doesn't take a list, I think you'll have to pass multiple arguments
```
 --exclude 'common.py' --exclude 'test_writers.py' 
```

----

pre-commit can be done in a separate PR actually, don't worry about it here
Perhaps it makes sense to filter the search for os.remove to specifically exclude the cases where it occurs currently, rather than exclude the whole files? If so any suggestions on preferred ways to achieve this?
thanks @jnecus
Gh 36562 typeerror comparison not supported between float and str
State: closed

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


Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations
name: MultiIndex

Comments:
@arw2019 comments addressed (most of them), please review.
Hello @ssche! 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-11-03 23:54:13 UTC
thanks @ssche very nice!
TST: Series constructor behavior is consistent for mixed float NaN and int dtype data
State: closed

- [x] closes #22585
- [x] 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 @arw2019 
um, why does this solve the issue in #22585?  IIUC the issue there was that Series(list_with_nan, dtype=inty) raises while Series(ndarray_with_nan, dtype=inty) coerces, so this is just testing the _inconsistent_ behavior
or maybe it was consistent at the time of this PR but the ndarray case has been changed since then?
CLN: in maybe_cast_to_integer_array assert that dtype arg is an integer dtype
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: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Clean

Comments:
thanks @arw2019
CI: remove xfail for numpy-dev on branch 1.1.x only
State: closed

partially reverts pandas-dev/pandas#35537, xref https://github.com/pandas-dev/pandas/issues/35481#issuecomment-705144719

Labels:
name: CI
description: Continuous Integration

Comments:
Not sure why still needed on master
looks like test_loc_setitem_empty_append_raises still fails. will update tomorrow.
just the travis-36-cov failures. this build was until recently timing out 100% of the time, it's now apparent there are a few errors there that need investigation.

Linux py37_np_dev passing so merging.
> just the travis-36-cov failures. this build was until recently timing out 100% of the time, it's now apparent there are a few errors there that need investigation.

opened #37309 for this.
Backport PR #37034 on branch 1.1.x (REGR: Fix casting of None to str during astype)
State: closed

Backport PR #37034: REGR: Fix casting of None to str during astype

Labels:
name: Strings
description: String extension data type and string data

Comments:
---------------------------------- Hypothesis ----------------------------------
Falsifying example: test_on_offset_implementations(
    dt=datetime.datetime(1900, 1, 1, 0, 0, tzinfo=<DstTzInfo 'US/Michigan' LMT-1 day, 18:28:00 STD>),
    offset=<5 * YearBegins: month=1>,
)
You can reproduce this example by temporarily adding @reproduce_failure('5.37.1', b'AAICSwAAAAAAAAAACAAABQA=') as a decorator on your test case
ENH: Add MultiIndex.dtypes
State: closed

- [x] closes #37062 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Adds the .dtypes property to MultiIndex using the suggested implementation

Labels:
name: Enhancement
name: MultiIndex

Comments:
@jreback Made the requested changes. Could you take a look?
can you merge master
> can you merge master

Done.
Hello @skvrahul! 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-12-08 18:24:25 UTC
I've resolved conflicts. Should be good to merge with the base branch now.
Made the requested changes and passing all tests. Should be okay to merge now
@jreback @arw2019 
thanks @skvrahul very nice!
BUG: preserve timezone info when writing empty tz-aware series to HDF5 
State: closed

- [x] closes #20594 (coupled with #37069 for empty tz-aware dataframes)
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

xref #20595 (stale PR that looked into this)

Labels:
name: IO HDF5
description: read_hdf, HDFStore
name: Timezones
description: Timezone data dtype

Comments:
> ok i am not sure how to grok this one as i think its on top of the other; in the future put them both in 1 PR is ok for this size change, or now, wait till the other is merged then rebase this one (or make this orthogonal)

Ok will do! For this one I'll wait until the other is merge (since this is a dependant)
@jreback this is ready for re-review
- [x] rebased against master (includes  #37069)
- [x] rewrote the tests from here and  #37069 using `tz_aware_fixture`
- [x] added whatsnew
CI Green + ready for re-review
thanks @arw2019
BUG: Resolve passing datetime strings of the form "1-01-01 00:00:00"
State: closed

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


Labels:

Comments:
As highlighted in https://github.com/pandas-dev/pandas/issues/37071#issuecomment-707484947, such a form is actually ambiguous from dateutil's point of view so I'll take this issue upstream to where the ambiguity arises (xarray). Happy for anyone else to pick up if they can see a better solution!
BUG: preserve timezone info when writing empty tz-aware frame to HDF5
State: closed

- [ ] partially closes #20594
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Will handle series case in follow-on

Labels:
name: IO HDF5
description: read_hdf, HDFStore
name: Timezones
description: Timezone data dtype

Comments:
looks like dupe of #37072 
sorry just read the comment on the other PR
Green.
thanks @arw2019
TYP: Misc groupby typing
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: Groupby
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
thanks @rhshadrach