BrandonFors/pandas-github-issues huggingface dataset:
REF: setitem_with_indexer DataFrame always go through split_path
State: closed

Still has some rough edges to round off, but putting it up bc i think it might be relevant for some of @jorisvandenbossche 's issues in getting AM behavior to match BM.

Sits on top of #39510

Labels:
name: Refactor
description: Internal refactoring of code
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves

Comments:
(@jorisvandenbossche if you were so inclined to make a copy-pasteable one-liner to run the relevant array-manager tests (could put it in the Makefile?) that'd make my life marginally better)
> Can you give a high-level summary of the changes?

ATM `iloc.__seitem__` (and through it, `loc.__setitem__`) go through different code paths depending on whether there is more than 1 Block (or if there is ArrayManager).  These code paths have different behaviors w/r/t copy/view semantics.

This PR does 2 things:

1) Have all 2D cases go through the "split" path that currently only multi-Block cases go through.

2) Within that split path, use try-inplace-fallback-to-cast behavior intended to match the non-split case.
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.
@jbrockmendel there are now some conflicts here to resolve
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 2021-11-30 04:09:54 UTC
rebased, have a couple test failures i need to figure out still
status?
> status?

One case that is still failing; I'm optimistic #43632 will make it easier to fix.
Mothballing while troubeshooting locally.
TYP: change ArrayLike/AnyArrayLike alias to Union
State: closed

follow-on from #36092

The first commit is the change in the alias.

Although this does get rid of many of the false positives, as well as removing ignores this also results is some changes in the mypy error codes and some new errors where we might need a typevar (or more likely an overload)

The subsequent commits were an attempt to ensure that this PR only removed ignores.

The PR is starting to get a bit difficult to review imo. so I have stopped for now.

I could break off the first commit and then submit the subsequent commits as separate PRs if it makes review easier.

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

Comments:
Watching progress on this so I can restart work from #39501
LGTM

@jreback let's prioritize this to avoid merge conflict pileups
STYLE: Extending codespell to pandas/tests/  part3  38802
State: closed

- [x] closes #38802
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [ ] whatsnew entry


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

Comments:
Done with all the changes.
Thanks @01-vyom
BUG: Post-merge fixup, raise on dimension-expanding
State: closed

Looks like a couple of individually-correct things got merged that together are causing failures on master.  This should fix it.

Labels:
name: CI
description: Continuous Integration
name: Internals
description: Related to non-user accessible pandas implementation

Comments:
cc @jorisvandenbossche  i'll do a quick PR for the mypy failures separately.
Thanks @jbrockmendel and @simonjayhawkins !
BUG: better exception on invalid slicing on CategoricalIndex
State: closed

After fixing the CategoricalIndex behavior (first commit), the "kind" keyword is no longer needed in _maybe_cast_slice_bound, so removed that.

After this the keyword is no longer needed in get_slice_bound, slice_locs, slice_indexer, but those are technically public so didnt rip them out.

Labels:
name: Categorical
description: Categorical Data Type
name: Clean

Comments:
cc @TomAugspurger docbuild looks like dask is using _maybe_cast_slice_bound

```
~/miniconda3/envs/pandas-dev/lib/python3.8/site-packages/dask/dataframe/indexing.py in _maybe_partial_time_string(index, indexer, kind)
    352     if isinstance(indexer, slice):
    353         if isinstance(indexer.start, str):
--> 354             start = index._maybe_cast_slice_bound(indexer.start, "left", kind)
    355         else:
    356             start = indexer.start

TypeError: _maybe_cast_slice_bound() takes 3 positional arguments but 4 were given
```
BUG: IntervalArray.insert cast on failure
State: closed

- [ ] closes #xxxx
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [ ] whatsnew entry

We fixed this for DTI/TDI/PI in #39068.  Also did CategoricalIndex recently, not sure where off the top.

Labels:
name: Bug
name: Interval
description: Interval data type

Comments:
does it need a whatsnew note?
> does it need a whatsnew note?

i think this is covered by the whatsnew note in #39068
DOC: Proper alignment of column names #40355
State: closed

- [x] closes #40355
- [ ] tests added / passed
- [ ] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [ ] whatsnew entry


Labels:
name: Docs

Comments:
Before and after

![image](https://user-images.githubusercontent.com/37313715/110698494-57f90a00-81b3-11eb-95fe-38146c50fd13.png)

thanks @mschmookler
BUG: copying from Jupyter table to excel misaligns structure
State: closed

- [x] closes #12147 

when copying a table from Jupyter to Excel the column header row is translated to the left because there is no value to copy.

![Screen Shot 2021-03-10 at 19 53 28](https://user-images.githubusercontent.com/24256554/110682247-019fc180-81db-11eb-81a4-f8d71b08181e.png)

this PR puts a non-breaking space in the empty cell so that structure is preserved.

![Screen Shot 2021-03-10 at 19 55 41](https://user-images.githubusercontent.com/24256554/110682369-2431da80-81db-11eb-9198-5e13661805cf.png)

my excel above is quite old, but this effect is worse on more recent excels since I believe they merge cells which have a rowspan (in the multiindex column headers) or a colspan (in a multiindex index labels). With columns merging in wrong place it is a bit of a nuisance. 

Labels:
name: Bug
name: Styler
description: conditional formatting using DataFrame.style

Comments:
seems fine, can you add a whatsnew note, ping on green
@jreback ping is green
thanks @attack68
Fix formatting in extractall example outputs
State: closed

I noticed that these doctests lost some formatting in a refactor. It's a little surprising the tests were still passing, presumably doctests is configured to ignore leading whitespace?

Labels:
name: Docs

Comments:
thanks @TheNeuralBit i don't think doctests care about the actual whitespace match (though its possible we also runing these specific ones).
I also modified the output and confirmed that it failed when running `./ci/code_checks.sh doctests` so I think you're right that its ignoring whitespace
PERF: repeated slicing along index in groupby
State: closed

This cuts another 25% off of the benchmark discussed https://github.com/pandas-dev/pandas/pull/40171#issuecomment-790219422, getting us to within 3x of the cython-path performance.

Labels:
name: Groupby
name: Performance
description: Memory or execution speed performance

Comments:
i think following today's call we're all set here?
guess i merged in wrong order. can you merge master and ping on green.
@jorisvandenbossche any guesses why the array manager sql tests are breaking on a bunch of PRs?

update: looks like they break for non-ArrayManager, just thats the only place we have it running with up-to-date sqlalchemy
Yes, it seems some new deprecation warnings with a new SQLAlchemy
green ex unrelated sqlalchemy issue
merging this, the sqlalchemy patch can be a followup
[ArrayManager] TST: Enable extension tests
State: closed

xref #39146


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Internals
description: Related to non-user accessible pandas implementation

Comments:
General note: I am using the `if hasattr(result._mgr, "blocks"):` to check whether we are using BlockManager or ArrayManager in the base extensions tests, to not rely on fixtures (like `using_array_manager`) that are defined higher up in pandas but are not available for downstream packages.
@jreback @jbrockmendel any comments here? (trying to get all testing PRs merged to reduce the custom selection)
Will take a look now.
looks fine, may want to rebase
[ArrayManager] TST: enable apply tests
State: closed

xref #39146

Labels:
name: Internals
description: Related to non-user accessible pandas implementation
name: Apply
description: Apply, Aggregate, Transform, Map

Comments:
@jreback @jbrockmendel any comments here? (trying to get all testing PRs merged to reduce the custom selection)
one question, otherwise LGTM.  azure failure looks unrelated.
Failure is an unrelated docstring validation issue that also happens on other PRs
DOC: fixup formula for exponentially weighted mean with input vector …
State: closed

…of times? #40327

- [x] closes #40327 
- [ ] tests added / passed
- [ ] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [ ] whatsnew entry


Labels:
name: Docs
name: Window
description: rolling, ewma, expanding

Comments:
Thanks, looks good. Could you render the doc and post a picture to confirm the change worked?
Old version and new version:

![image](https://user-images.githubusercontent.com/37313715/110594968-13358a80-8143-11eb-99ee-4523d6dbb436.png)

Thanks @mschmookler !
CLN: remove unused c_encoding
State: closed

`c_encoding` has always been `NULL` since #36997. This allows removing/simplifying a lot of code in pandas/_libs/parsers.pyx.

Labels:
name: Unicode
description: Unicode strings
name: IO CSV
description: read_csv, to_csv
name: Clean

Comments:
nice @twoertwein 

any perf gains from this?
> any perf gains from this?

I didn't check, but I wouldn't expect a significant change as it mostly removes dead code. I can run the benchmarks later today.
BUG: RollingGroupby no longer keeps the groupby column in the result
State: closed

- [x] closes #32262
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry

Borrowed some work from https://github.com/pandas-dev/pandas/pull/32332

Labels:
name: Bug
name: Window
description: rolling, ewma, expanding

Comments:
cc @pandas-dev/pandas-core, @pandas-dev/pandas-triage  if any comments (this was a discussion item on the call i think month before last)

https://github.com/pandas-dev/pandas/issues/38787 was the discussion item I believe you are referring to @jreback which this PR does not address
thanks @mroeschke
TYP: Add typing for remaining IO XML methods with conditional for lxml
State: closed

- [X] references #40131
- [X] tests passed
- [X] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [ ] whatsnew entry (not necessary)


Labels:
name: Typing
description: type annotations, mypy/pyright type checking
name: IO XML
description: read_xml, to_xml

Comments:
this is fine, ping when CI finishes and greenish (failure is being fixed elsewhere)
thanks @ParfaitG
BUG: fix regression in Styler.export/use
State: closed

- [x] closes #39735

#39396 solved a bug that CSS was duplicated everytime a Styler rendered. The solution maintained the computed `ctx` object and cleared the `_todo` list of methods so it wasn't recalculated. This broke the `export` and `use` methods since they rely on passing methods from the `_todo` list.

This PR solves the above duplication problem by clearing the `ctx` object before each `render`, maintaining the `_todo` from one render to another, and therefore fixes the broken methods. 


Labels:
name: Regression
description: Functionality that used to work in a prior pandas version
name: Styler
description: conditional formatting using DataFrame.style

Comments:
is there anything user visible that would have changed? can you make a test that exercises this? (i see you change the internal test which is good as well).
No the `ctx` obj is no documented, is complex and the user should never, and is unlikely, to be using it directly.

There was an existing `test_export`, it just didn't pick up the case where a `Styler` had been rendered so now this is modified.
cool thxs
STYLE: Extending codespell to pandas/tests Part 2
State: closed

- [X] xref #38802 
- [X] tests added / passed
- [X] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [ ] whatsnew entry


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

Comments:
doesn't this have to turn on this in the codespell config? (or is this a precursor)
> doesn't this have to turn on this in the codespell config? (or is this a precursor)

Yes @jreback, by removing the exclude from the codespell config, we get 68 file changes so we decided to split those changes into 3 PRs and include the codespell config change in the final PR in-order to pass the pre-commit for all the PRs.

More Info here: #40320 
Made all the necessary changes.
Thanks @01-vyom !
REF/TYP: indexes
State: closed

The only non-trivial change here is changing `_id` from `_id = _Identity(object)` to `_id = object()`, which I found in profiling (https://github.com/pandas-dev/pandas/pull/40171#issuecomment-790219422) makes a non-trivial difference as the non-cython performance inches closer to the cython performance.

Share DTI/TDI/PI's _simple_new with Categorical.

Simplify DTI/TDI `__contains__`

Annotations.

Docstring follow-up to #40241

Labels:
name: Index
description: Related to the Index class or subclasses
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
> The only non-trivial change here is changing _id from _id = _Identity(object) to _id = object(), which I found in profiling (#40171 (comment)) makes a non-trivial difference as the non-cython performance inches closer to the cython performance.

what kind of gains here?
hmm, the azure checks are not running. odd
> what kind of gains here?

on the benchmark ive been using, _reset_identity is about 2.4% of the runtime, which this cuts to 1%
thanks @jbrockmendel
[ArrayManager] TST: run (+fix/skip) pandas/tests/series/indexing tests
State: closed

xref #39146

Getting the tests passing for the indexing tests, her for `pandas/tests/series/indexing` (`pandas/tests/indexing` is done in https://github.com/pandas-dev/pandas/pull/40325 and `pandas/tests/frame/indexing` is done in https://github.com/pandas-dev/pandas/pull/40323)

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Internals
description: Related to non-user accessible pandas implementation

Comments:
LGTM cc @jreback
thanks @jorisvandenbossche
[ArrayManager] TST: run (+fix/skip) pandas/tests/indexing tests
State: closed

xref #39146

Extracting part of the test changes of https://github.com/pandas-dev/pandas/pull/39578. To keep the size a bit more limited, I am splitting the PRs, so this PR is doing `pandas/tests/indexing`. The `pandas/tests/frame/indexing` are handled in https://github.com/pandas-dev/pandas/pull/40323



Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Internals
description: Related to non-user accessible pandas implementation

Comments:
So one of the main causes for differences in behaviour between BlockManager and ArrayManager, is that ArrayManager always takes the "split" path in the indexing code. And, the split path has different behaviour for several cases compared to the single block path. Which is the case for both BlockManager and ArrayManager, but because many of the tests are using simple dataframes, the default tests with BlockManager don't catch this. 

For example, setting values with `.loc[:, col]` still overwrites the column instead of updating inplace when possible.

I still need to open issues for those several cases that fail for BlockManager as well.
> So one of the main causes for differences in behaviour between BlockManager and ArrayManager, is that ArrayManager always takes the "split" path in the indexing code

FWIW ive got a branch nearing readiness that makes setitem_with_indexer take the split_path iff it is 2D, which will hopefully make keeping these behaviors in sync somewhat easier.
one question, LGTM pending green
BUG: Fix sort_values for empty by argument
State: closed

- [x] closes https://github.com/pandas-dev/pandas/issues/40258
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry


Labels:
name: Bug
name: Algos
description: Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff

Comments:
thanks @dsaxton
[ArrayManager] TST: run (+fix/skip) pandas/tests/frame/indexing tests
State: closed

xref #39146

Extracting part of the test changes of https://github.com/pandas-dev/pandas/pull/39578. This PR is doing `pandas/tests/frame/indexing`. The `pandas/tests/indexing` tests are handled in https://github.com/pandas-dev/pandas/pull/40323.


Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Internals
description: Related to non-user accessible pandas implementation

Comments:
@jbrockmendel ready to merge here?
LGTM cc @jreback
thanks @jorisvandenbossche
STYLE: Extending codespell to pandas/tests
State: closed

- [X] xref #38802
- [X] tests added / passed
- [X] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [ ] whatsnew entry


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

Comments:
Nice, thanks @01-vyom for doing this!

This changes lots of files though, could you please split it up into smaller PRs? Maybe 3 PRs doing about 20 files each?
Ok, will do that
Made all the necessary changes.
I made the second PR with other files, as both of these PRs are merged I will make the final PR.
Nice, thanks! There's some (unrelated to this PR) issue with CI though, will revisit once that's fixed
Hi @01-vyom can you fetch and merge upstream/master please?
Thanks @01-vyom !
STYLE use pd_array in core
State: closed

As requested by @jbrockmendel [here](https://github.com/pandas-dev/pandas/pull/40150)

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

Comments:
@MarcoGorelli i think an easier solution is

construction.py:
```
def pd_array(...):
```

\_\_init\_\_.py:
```
from pandas.core.construction import pd_array as array
```
For me this is fine (but it's @jbrockmendel that suggested otherwise)

The linting error seems relevant.
joris makes a reasonable point about `repr(pd_array)`.  im fine with the approach in this PR
> The linting error seems relevant.

Indeed - fixed, thanks!
@MarcoGorelli can you do another (final) merge of master to be sure there are no new failures?
lgtm. @MarcoGorelli can you merge master and ping on greenish
@MarcoGorelli if you can merge master once more
thanks @MarcoGorelli
PERF: reduce overhead in groupby _cython_operation
State: closed

The `BaseGrouper._cython_operation` has lots of checking of the dtype etc, which gives quite some overhead compared to the actual aggregation function (for small data). This is a first set of small changes to cut down that overhead.

Using the `GroupManyLabels` benchmark we have 

```python
ncols = 1000
N = 1000
data = np.random.randn(N, ncols)
labels = np.random.randint(0, 100, size=N)
df = DataFrame(data)
df_am = df._as_manager('array')
```

gives

```
In [2]: %timeit df_am.groupby(labels).sum()
48.9 ms ± 1.92 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)  <-- master
39.1 ms ± 695 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)  <-- PR
```

Labels:
name: Groupby
name: Performance
description: Memory or execution speed performance

Comments:
can you also report how the non-AM perf is affected?
> can you also report how the non-AM perf is affected?

For the benchmark, it's not really affected. The overhead I am optimizing here is only around 1% for the BlockManager case. It's only when doing those checks 1000x instead of 1x that this becomes a noticeable bottleneck (the benchmarks has 1000 columns / single block). 

I assume that for a small Series, the overhead might also be noticeable for non-AM. But trying this, and I don't really see much difference, and that's because for a single column, the dominant time is the actual factorization (which for 1000 columns is also only done once). Using the code from above:

```
In [2]: %timeit df[0].groupby(labels).sum()
384 µs ± 4.76 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)  <-- master
374 µs ± 4.18 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)  <-- PR
```
(some failing tests related to (unsupported) operations on categorical column that I still need to address)
This is all passing now
LGTM ex one requested annotation, cc @jreback
ERR: Raise NotImplimentedError for EWMA with times and adjust=False
State: closed

- [x] xref #40098
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry


Labels:
name: Error Reporting
description: Incorrect or improved errors from pandas
name: Window
description: rolling, ewma, expanding

Comments:
thanks @mroeschke
API: Add `Styler.to_html`, for saving output to HTML file
State: closed

- [x] closes #11700 and closes #13379

## `Styler.to_html` method

This PR adds the above method to `Styler`. 

This is a **minimal** introduction with limited arguments to provide just the core file io functionality and basic options, either to write a fully structured HTML file or just the `Styler.render` output. It is very easier to extend in follow ons.

#### Jinja 2 template inheritance

Due to the more complex logic, the jinja2 templates were split into smaller components:

 - html_style.tpl: this just deals with the `<style>` html element
 - html_table.tpl: this just deals with the core `<table>` element including (or excluding) style identifiers
 - html.tpl: this is the controlling template which imports the above based on what is required.

It is more performant to have a separate temple that excludes styling elements by about 33%, rather than include the logic in a single template. I think it is easier to maintain as well.

#### Tests

A new `test_to_html.py` file is added. Some tests are moved from `test_style.py` if they more relate to HTML generation. Some new tests are added.

#### Objective

Ultimately the medium-term aim is to deprecate `DataFrame.to_html`. `Styler` is currently faster at rendering HTML than `DataFrame.to_html` (#39972), and this is further improved in this PR.
There is a standalone purpose for wanting to create HTML to file from Styler.
This PR complements `Styler.to_latex` whose aim is also to deprecate `DataFrame.to_latex`.


Labels:
name: IO HTML
description: read_html, to_html, Styler.apply, Styler.applymap
name: Styler
description: conditional formatting using DataFrame.style

Comments:
@jreback ping: greenish, failure unrelated
thanks @attack68