BrandonFors/pandas-github-issues huggingface dataset:
BUG: Too aggressive typing in NDFrame.align
State: closed

- [x] closes #31785

The type checking was too aggressive. ``right`` has type ``Any``, so the wrapping in ``_ensure_type`` should not be done.

Labels:
name: Regression
description: Functionality that used to work in a prior pandas version
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
> The type checking was too aggressive

Is this in reference to annotations or to runtime type checking?
> Is this in reference to annotations or to runtime type checking?

It's type checking, but can spill over to the run time because an ``assert`` is used to enforce the type, (that's what the call to ``._ensure_type`` does). Without ``._ensure_type`` the type of left would be ``Optional[DataFrame]`` and mypy would complain of operations on None.


Do any of the other usage of `_ensure_types` introduced in the PR also need fixing?
There should`t be more; here I assumed that left and right should be treated alike, which was a wrong idea.
Ok, I've added a test.
thanks can we open an issue about how to improve / fix _ensure_type to make things like this work?
REF: remove iloc case from _convert_slice_indexer
State: closed

_convert_slice_indexer is turning out to be one of the stickier methods to figure out, xref #31658.

The case with kind=None is only called from one place in core.indexing, and that is not reached in the tests.  I'd like to either a) figure out a non-None kind to pass to it, or b) determine that it can never be reached so can be removed.  Any thoughts on this are very welcome.

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

Comments:
sure
sure
TST: parametrize some indexing tests
State: closed

2/many

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:
> ok for now, or can do more here, lmk.

Agreed with all your comments.  These tests are a mess, (xref #31813, #31816) so id like to get these cleanups in small pieces to avoid merge headaches
thanks
replaced old format strings
State: closed

Changed the string formatting to f-strings.
ref #29547 
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`


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

Comments:
Thanks for the PR - Please run black on your changes:

```
Checking black formatting
would reformat /home/runner/work/pandas/pandas/pandas/tests/internals/test_internals.py
would reformat /home/runner/work/pandas/pandas/pandas/tests/tseries/frequencies/test_inference.py
```
@UnforgivenSoul can you fix merge conflict and address comments?
Closing as stale
Implement some reductions for string Series
State: closed

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

Labels:
name: Strings
description: String extension data type and string data
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
Just as a counterpoint do we really want to support all of these? `sum` in particular is strange to me to support on a string dtype
The value of `sum` is certainly less clear, but I think `min`/`max` are nice to have? 
> Just as a counterpoint do we really want to support all of these? `sum` in particular is strange to me to support on a string dtype

Agreed summing strings is a little odd, but is it worth implementing for the sake of consistency with Series of object dtype (for which this is a valid operation)?
I don’t think consistency with object dtype is a goal for the string dtype. Even for min/max I’m not sure what those mean in a lot of cases, unless the answer is to fallback to Python semantics. 

My concern is I think that as an answer conflicts with the goal of creating a native string type
> I don’t think consistency with object dtype is a goal for the string dtype. Even for min/max I’m not sure what those mean in a lot of cases, unless the answer is to fallback to Python semantics.
> 
> My concern is I think that as an answer conflicts with the goal of creating a native string type

Fair point about consistency not being important. I do think though that strings having an "order" to them is a pretty useful / natural concept (we'd probably want to allow sorting of strings, in which case we'd also want min and max)
>  Even for min/max I’m not sure what those mean in a lot of cases, unless the answer is to fallback to Python semantics.

You can sort strings, and then min/max have a rather clear meaning IMO

But it's true we certainly don't need to do this for consistency with object dtype, but because we think it is useful 
(we only need to check / fix consistency between dataframe vs series operation)
i would certain add operations that work now on object types - otherwise the dtypes won’t be used generally which is not great 
More thoughts on adding `min`/`max` and/or `sum`. I would like to see min/max added, but care less about `sum`.
moving this off 1.0.2 as this has raised some non-trivial to solve issues. if you want a bug fix out of this ok, but then this needs to decouple the issues.
Going to close this for now since it looks to be quite a bit larger than I originally thought (seems like fixing nanmin / nanmax might be the right approach).

I'm not sure how best to implement this, but another idea might be to fill with an arbitrary non-NaN value from each array along the given axis before taking the min or max, then I think it should work for any dtype (although might be a little slower).
BUG Decode to UTF-8 the dtype string read from a hdf file
State: closed

Fixes GH31750

The dtype value wasn't being decoded to `UTF-8` when reading a DataFrame
from a hdf file. This was a problem when reading a hdf that was
created from python 2 with a fixed format as the dtype was being read as `b'datetime'`
instead of `datetime`, which caused `HDFStore` to read the data as
`int64` instead of coercing it to the correct `datetime64` dtype.

- [x] closes #31750
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: IO HDF5
description: read_hdf, HDFStore

Comments:
thanks
thanks folks
@jreback to get this fix backported to 1.0.2, do I need to open a second PR targeting that branch, or is it going to get picked up automatically by that backport bot?
we don’t backport regular big fixes
they come out in the next major release 
we don’t backport regular big fixes
they come out in the next major release
BUG: Fixed encoding of pd.NA with to_json
State: closed

- [x] closes #31615
- [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
name: IO JSON
description: read_json, to_json, json_normalize

Comments:
@WillAyd I've ran the full asv suite, and then re-ran any asv tests that had a ratio >1.1

There are no significant changes from this PR.

Let me know if you want any more information or results files.
@WillAyd I ran the entire suite.  Whenever I run the entire suite I always get false positives, so I rerun anything that shows up on the first go-around.

There are no significant performance differences in io.json.ToJSON (or anywhere else).
@ArtificialQualia Thanks a lot!
Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

```
$ git checkout 1.0.x
$ git pull
```

2. Cherry pick the first parent branch of the this PR on top of the older branch:
```
$ git cherry-pick -m1 0bfb8cbdb7bac29d158a868817f606145bf72f06
```

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

```
$ git commit -am 'Backport PR #31748: BUG: Fixed encoding of pd.NA with to_json'
```

4. Push to a named branch :

```
git push YOURFORK 1.0.x:auto-backport-of-pr-31748-on-1.0.x
```

5. Create a PR against branch 1.0.x, I would have named this PR:

> "Backport PR #31748 on branch 1.0.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to [suggest an improvement](https://github.com/MeeseeksBox/MeeseeksDev).
TST: expand tests for ExtensionArray setitem with nullable arrays
State: closed

Follow-up on https://github.com/pandas-dev/pandas/pull/31484 to add some more test cases (eg also test the validation done in the `check_array_indexer` (wrong length, missing values), as well for nullable integer arrays), similarly to the tests I added for `__getitem__`.

cc @charlesdong1991 

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
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
needs rebase
Backport PR #31723 on branch 1.0.x (DOC: Add 1.0.2 whatsnew)
State: closed

Backport PR #31723: DOC: Add 1.0.2 whatsnew

Labels:

Comments:
Restarted Travis CI
I think this is https://github.com/pandas-dev/pandas/issues/31739, and in any case unrelated to the doc change, so merging
BUG: list-like to_replace on Categorical.replace is ignored or crash
State: closed

- [X] closes #31720
- [X] tests added / passed
- [x] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry

Covers the case where `to_replace` is a list-like and `value` is a string. Other cases, like "`to_replace` is dict and `value` is None", or "`to_replace` and `value` are both lists" are handled earlier in generic.py

Labels:
name: Regression
description: Functionality that used to work in a prior pandas version
name: Categorical
description: Categorical Data Type

Comments:
How does this behave when `to_replace` is already a dict? On master the replacement seems to work but the data type changes:

```python
In [5]: s                                                                                                                                                             
Out[5]: 
0    0
1    1
2    2
dtype: category
Categories (3, int64): [0, 1, 2]

In [6]: s.replace({0: 1})                                                                                                                                             
Out[6]: 
0    1
1    1
2    2
dtype: int64
```
@dsaxton this is because in `NDFrame` (generics.py) it chooses to not dispatch the dict case to the `self._data.replace`, but `self._data.replace_list` which takes a more complex path to iterate over its blocks and would need more drastic refactoring (it pre-computes masks to avoid repeated replacement if the same value appears more than once in `to_replace` or `value`). I think that is outside the scope of the current ticket
Thanks @JustinZhengBC ! 

Overall looks good
Hello @JustinZhengBC! 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-02-17 00:22:52 UTC
@jreback I've moved the whatsnew and addressed the other feedback
also pls merge master
@jreback green
thanks @JustinZhengBC very nice!
Add --quiet option to isort command
State: closed

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


Labels:
name: CI
description: Continuous Integration

Comments:
What does the output look like whenever an import is incorrectly sorted? Let’s push a commit here (you can mess up import ordering in any file) so we can double check all looks as expected on github actions
@alimcmaster1 done. the output looks like this in case of an error:

```
Check import format using isort
##[error]ERROR: /home/runner/work/pandas/pandas/pandas/_config/config.py Imports are incorrectly sorted.
Check import format using isort DONE
```
LGTM - thanks. cc @datapythonista 
@datapythonista The errors are still displayed. 
[This](https://github.com/pandas-dev/pandas/pull/31733#issuecomment-582933506) is a snippet from the output of [this](https://github.com/pandas-dev/pandas/pull/31733/checks?sha=4e72237f340eb63968f4bb099fec272eb6ab5769) commit where I changed the order of the imports to verify that errors are still being displayed (the CI logs seem to have been deleted, unfortunately)
Thanks for researching and fixing this!
fixed mypy errors in mypy-pandas.tests.extension.decimal.test_decimal
State: closed

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


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
lgtm outside of @jbrockmendel comments
BUG: Fix to_excel writers handling of cols
State: closed

- [x] closes #31677
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry - pending https://github.com/pandas-dev/pandas/pull/31723

Ref: https://github.com/pandas-dev/pandas/pull/31723

cc. @WillAyd @jbrockmendel 

Labels:
name: Regression
description: Functionality that used to work in a prior pandas version
name: IO Excel
description: read_excel, to_excel

Comments:
Hello @alimcmaster1! 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-02-06 21:26:42 UTC
lgtm
Can you add a whatsnew for 1.0.2?
thanks @alimcmaster1
API/BUG: make .at raise same exceptions as .loc
State: closed

- [x] closes #31722
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

This also (very) indirectly addresses #31683 which in turn will let us get rid of `CategoricalIndex.get_value` altogether.

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

Comments:
> Can you check some timings for at? (as this is supposed to be the fast one)

Yah we take a hit

```
In [2]: ser = pd.Series(range(10000))                                           
In [3]: %timeit ser.at[501]                                                     
4.54 µs ± 139 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)  # <-- master
7.8 µs ± 75.2 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)  # <-- PR

In [4]: df = ser.to_frame("A")                                                  
In [5]: %timeit df.at[501, "A"]                                                 
3.9 µs ± 25.4 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)  # <-- master
7.71 µs ± 99.4 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)  # <-- PR
```

We might get some of that back from #31709.
> How does this indirectly also address #31683 ? And if it does, shouldn't there be tests involving categorical / a whatsnew?

With this in place, all the places from which index.get_value are called are preceeded by maybe_cast_scalar_indexer (except one in which the key is a tuple).  That makes the call inside CategoricalIndex.get_value unnecessary, which addresses the motivating issue for #31683.  The "indexing on CategoricalIndex is hard" problem is still a problem.
thanks @jbrockmendel 
@jreback maybe you didn't notice, but *this time* I had a "request changes" open, as you asked previous time to indicate that you shouldn't merge without me having the chance to answer back. 
If there is still discussion going on, please give it some time.
@jbrockmendel can you do a follow up with:

- fixing the performance regression
- fixing the usability regression in the error message
> fixing the usability regression in the error message

Looking back at it, it's actually mainly the long fully qualified class name that is annoying, so that is an easy fix -> https://github.com/pandas-dev/pandas/pull/31769
DOC: Add 1.0.2 whatsnew
State: closed

- Need this for PR https://github.com/pandas-dev/pandas/issues/31677

cc. @TomAugspurger @jorisvandenbossche 

Labels:
name: Docs

Comments:
Can you use the same format (the sections) as the v1.0.1 file ?
And you can also already fill in the "contributor" (see the v1.0.1 file for how it is done there)
Thanks - Done @jorisvandenbossche 
@alimcmaster1 I removed the deprecation section as well, as normally there shouldn't be any deprecations (in 1.0.1 it was a special case to fix a regression that it was restored but also deprecated)
Thanks!
> @alimcmaster1 I removed the deprecation section as well, as normally there shouldn't be any deprecations (in 1.0.1 it was a special case to fix a regression that it was restored but also deprecated)

thanks make sense!
fix type errors in pandas/tests/extension/json/array.py
State: closed

Part of #28926

- [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:
nitpick: when naming these PRs, "fix type errors" makes me think there is a TypeError being raised somewhere.  something involving "annotations" or "mypy" might be more clear
restarted the azure build
Thanks @SaturnFromTitan
fix mypy errors in pandas/tests/arithmetic/test_datetime64.py
State: closed

Part of #28926
- [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:
@SaturnFromTitan can you merge master.
REF: Index.get_value call self.get_loc instead of self._engine.get_loc
State: closed

This makes Index.get_value match ExtensionIndex.get_value, so we can remove the latter.

Along with implementing _should_fallback_to_positional, this allows us to rip out Float64Index.get_value.

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

Comments:
thanks
DOC Adds newline to dataframe melt
State: closed

Adds newline before versionadded in `dataframe.melt` so that sphinx/rst can pick it up and render it correctly.

- [ ] closes #xxxx
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Labels:
name: Docs

Comments:
Hello @thomasjpfan! 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-02-05 18:22:53 UTC
Great thanks @thomasjpfan
CLN: _convert_scalar_indexer only handle "loc" and "getitem"
State: closed

`_convert_scalar_indexer` is called with kind="iloc" from only one place, and in that case

1) the base class method is equivalent to just the 1-liner `self._validate_indexer("positional", key, "iloc")`
2) all subclasses just call the base class method

So by inlining that 1-liner, we can take the "iloc" case out of `_convert_scalar_indexer` altogether.

kind=None is never passed, so we can rip that right out.

Ultimately I want to disentable/de-duplicate/disambiguate `_convert_scalar_indexer` vs `_maybe_cast_indexer`

Partial overlap with #31625.

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

Comments:
can you rebase
rebased+green
started to fixturize pandas/tests/base
State: closed

Part of #23877
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

-------------------------------------------
I didn't apply the fixtures to the rest of `test_ops.py` yet since @jreback voiced some [concerns in this comment](https://github.com/pandas-dev/pandas/pull/30147#discussion_r356587642).

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

Comments:
I think I addressed all the review comments. Friendly ping @jreback @WillAyd @jbrockmendel 
xref: #31989
DOC: Removed Notes from DataFrame.applymap
State: closed

- closes #31620 

Documentation screenshot: 
![Screenshot from 2020-02-05 17-08-33](https://user-images.githubusercontent.com/39258575/73838709-5172c300-483a-11ea-96b4-6faa1918b73f.png)


Labels:
name: Docs

Comments:
Please review this @TomAugspurger 
Thanks @r0cketr1kky 

Looks like you've checked in some unwanted files (e.g.  `doc/make.spec`) - could you please [unstage them](https://docs.gitlab.com/ee/university/training/topics/unstage.html)?
Hello @r0cketr1kky! 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-31 13:43:31 UTC
Please review this!
This seems stale. @r0cketr1kky if you want to finish this (mainly remove the unrelated changes), please open a PR or ping us here to reopen. Thanks!
Sorry for being late! I just noticed this @datapythonista ! Can you reopen this? I'll revert back with the changes.
@r0cketr1kky can you merge master and update
It doesn't look to me like this issue is actually solved.
I hope to get it solved in #34183
Closing as #34183 was merged and closed the issue - congrats @alonme for a really impressive contribution!