BrandonFors/pandas-github-issues huggingface dataset:
API: pseudo-public internals API for downstream libraries
State: closed

Downstream libraries accessing core.internals causes headaches pretty regularly  (https://github.com/pandas-dev/pandas/pull/40149#discussion_r585699450, #38134,).  The proposal: define a public-ish stable-ish API to expose for these libraries, leaving us free-er to make changes in internals.

This exposes `make_block`.  I expect we'll also need to expose one or both of `create_block_manager_from_arrays`/`create_block_manager_from_blocks` (comment in `internals.__init__` says they are there for downstream xref #33892)


Labels:
name: API Design
name: Internals
description: Related to non-user accessible pandas implementation
name: API - Consistency
description: Internal Consistency of API/Behavior

Comments:
> can you add a test which exercises this from a downstream author POV (e.g. create an manipulate things, e.g. like what pyarrow is doing), but only do this by importing the new api you are establishing so they can't cheat

i think the way to do this is to rename the internal version, then our existing downstream/io tests will test for this
Thanks, it's probably a good idea to make a public-ish `make_block`, so you are more free to make changes/optimizations to the one used internally. Now, then we do need to add tests for this, because it won't longer be tested through our own test suite.

In pyarrow, we currently make use of `BlockManager(..)` constructor as well. And further, we access `CategoricalBlock`, `DatetimeTZBlock` etc, but only as object, eg for `isinstance(block, ObjectBlock)` (never calling the `__init__` of those). Now, I suppose that we could remove this usage (instead of checking for CategoricalBlock, we could also check that `block.values` is a Categorical, for example)
CLN: Refactor tests and delete duplicates
State: closed

- [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


Deleted a few duplicates and parametrized 

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

Comments:
thanks @phofl
Make Index.all compatible with numpy.all
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
- [x] whatsnew entry

Due to a bug in my own code, I discovered that `pandas.Index` objects cannot be passed to the `numpy.all` function:

```python
In [1]: import numpy as np

In [2]: import pandas as pd

In [3]: idx = pd.Int64Index([1,2,3,4,5])

In [4]: np.all(idx)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-597ac8c43e48> in <module>
----> 1 np.all(idx)

<__array_function__ internals> in all(*args, **kwargs)

~/venvs/funpack.venv/lib/python3.7/site-packages/numpy/core/fromnumeric.py in all(a, axis, out, keepdims, where)
   2437     """
   2438     return _wrapreduction(a, np.logical_and, 'all', axis, None, out,
-> 2439                           keepdims=keepdims, where=where)
   2440
   2441

~/venvs/funpack.venv/lib/python3.7/site-packages/numpy/core/fromnumeric.py in _wrapreduction(obj, ufunc, method, axis, dtype, out, **kwargs)
     83                 return reduction(axis=axis, dtype=dtype, out=out, **passkwargs)
     84             else:
---> 85                 return reduction(axis=axis, out=out, **passkwargs)
     86
     87     return ufunc.reduce(obj, axis, dtype, out, **passkwargs)

TypeError: all() got an unexpected keyword argument 'axis'
```

Using [this PR](https://github.com/pandas-dev/pandas/pull/26324/) as precedent, the fix is quite straightforward. 


Labels:
name: Bug
name: Compat
description: pandas objects compatability with Numpy or Python functions
name: Index
description: Related to the Index class or subclasses

Comments:
@jreback Requested changes made - thanks!
thanks @pauldmccarthy very nice!
PERF: cache _get_cython_function in groupby ops
State: closed

Looking up the exact cython function is relatively costly, and is also something that is done repeatedly for a limited number of possible cases, so a good target for caching. 

Using the `GroupManyLabels` benchmark we have:

https://github.com/pandas-dev/pandas/blob/b835ca2fc2f772c27c914ae532cd32f8db69724a/asv_bench/benchmarks/groupby.py#L133-L145

```python
ncols = 1000
N = 1000
data = np.random.randn(N, ncols)
labels = np.random.randint(0, 100, size=N)
df = pd.DataFrame(data)
df_am = df._as_manager('array')
```

I get

```
In [2]: %timeit df_am.groupby(labels).sum()
72.6 ms ± 1.24 ms per loop (mean ± std. dev. of 7 runs, 10 loops each) <-- master
66.4 ms ± 1.43 ms per loop (mean ± std. dev. of 7 runs, 10 loops each) <-- PR
```

(the difference is not huge, but I am going to do several PRs with small changes that give a few ms improvement. And with all those together I hope to get the total time down to around 20ms, and in that case 5ms is significant)



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

Comments:
LGTM
PERF: don't sort data twice in groupby apply when not using libreduction fast_apply
State: closed

See https://github.com/pandas-dev/pandas/pull/40171#issuecomment-789120453 for context, noticed that we were calling `splitter._get_sorted_data()` twice when using the non-fast_apply fallback.

Using the benchmark case from `groupby.Apply.time_scalar_function_single/multi_col` (like in https://github.com/pandas-dev/pandas/pull/40171#issuecomment-789116039), but then with bigger data (`10 ** 6` instead of `10 ** 4`):

```
N = 10 ** 6
labels = np.random.randint(0, 2000, size=N)
labels2 = np.random.randint(0, 3, size=N)
df = DataFrame(
    {
        "key": labels,
        "key2": labels2,
        "value1": np.random.randn(N),
        "value2": ["foo", "bar", "baz", "qux"] * (N // 4),
    }
)
df_am = df._as_manager("array")
```

I get 

```
In [2]: %timeit df_am.groupby("key").apply(lambda x: 1)
252 ms ± 17.2 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)  <-- master
166 ms ± 5.7 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)  <-- PR
```

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

Comments:
cool. can you add a whatsnew note about perf :->
thanks @jorisvandenbossche (prob with future PRs can just add the issue numbers onto this perf for group note, but up 2 you)
REF: remove internal Block usage from FrameColumnApply
State: closed

In the `apply` code, there is still one case where we are directly using the internal blocks. This moves that to a method on the manager, so we don't have to interact directly with the blocks (which ideally we don't want outside of the internals, and let's ArrayManager cleanly support this as well). 

For the rest it still updates the Series' values in place, preserving the performance trick.

cc @jbrockmendel @rhshadrach 

Labels:
name: Refactor
description: Internal refactoring of code
name: Internals
description: Related to non-user accessible pandas implementation
name: Apply
description: Apply, Aggregate, Transform, Map

Comments:
small comments, otherwise LGTM
BUG: EWM.__getitem__ raised error with times
State: closed

- [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


Also somewhat ameliorates the performance hit in https://github.com/pandas-dev/pandas/pull/40072#issuecomment-787456038

```
dtype="float"
window=1000
N = 10 ** 5
arr = (100 * np.random.random(N)).astype(dtype)
ewm = pd.Series(arr).ewm(halflife=window)

%timeit ewm.mean()
# Master
2.59 ms ± 25.5 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

# PR
2.5 ms ± 31.5 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
```

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

Comments:
does this address https://github.com/pandas-dev/pandas/issues/40099?
> does this address #40099?

No, that issue has the expected output given the formula we're using but needs a discussion whether we want to make an exception in that case
thanks @mroeschke
BUG: Add decl when xml_declaration=True and pretty_print=False for et…
State: closed

…ree parser

- [X] references IO XML tracker #40131
- [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 (not necessary,  module releases: v1.3.0)


Labels:
name: IO XML
description: read_xml, to_xml

Comments:
thanks @ParfaitG
STYLE: Inconsistent namespace - dtypes (pandas-dev#39992)
State: closed

- [x] xref #39992 
- [ ] 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:
Thanks @parkerashlan
STYLE: incondintent namaspace(series)
State: closed

- [x] xref #39992
- [ ] 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: Code Style
description: Code style, linting, code_checks

Comments:
Some tests is failing, but I think these tests is not relevant to this change same to #40058.
Thanks @na2shell - yes, the failures from tests/extension are unrelated
CI: npdev SystemError
State: closed

cc @seberg was there a change in the ufunc code in the last day or so that might lead to a TypeError getting turned into a SystemError?

Labels:

Comments:
Hmm, yeah, it could be https://github.com/numpy/numpy/pull/18450 which is backported to the 1.20.x branch, so we may want to take care here.

I would expected the opposite, SystemError getting turned into TypeError (i.e. bad/weird error being made OK), if the opposite happened here, we should look into it maybe. Do you have a reproducer?!
> Do you have a reproducer?!

https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=55279&view=logs&j=eab14f69-13b6-5db7-daeb-7b778629410b&t=ce687173-08c6-5301-838d-71b2dda24510&l=43

I haven't tried it locally, but this is a copy-pasteable version of whats failing on the CI.

```
import pandas as pd
import pandas._testing as tm

data = pd.array([True, True, pd.NA, pd.NA, False, False, True])
df = pd.DataFrame({"A": [1, 1, 2, 2, 3, 3, 1], "B": data})
result = df.groupby("A").B.apply(lambda x: x.array)
expected = pd.Series(
            [
                df.B.iloc[[0, 1, 6]].array,
                df.B.iloc[[2, 3]].array,
                df.B.iloc[[4, 5]].array,
            ],
            index=pd.Index([1, 2, 3], name="A"),
            name="B",
        )
tm.assert_series_equal(result, expected)
```
Thanks, that was perfect, already found the issue. (If you run it in a python debug version, by default this already causes an assert, and that means you immediately get a gdb stacktrace, which trivially finds the issue – it was a simple oversight in that PR I linked.)
TYP: internals.construction
State: closed

- [ ] closes #xxxx
- [ ] 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: Typing
description: type annotations, mypy/pyright type checking
name: Constructors
description: Series/DataFrame/Index/pd.array Constructors

Comments:
Would it be OK to wait on https://github.com/pandas-dev/pandas/pull/39991 for this?
> Would it be OK to wait on #39991 for this?

should be OK
Thanks, https://github.com/pandas-dev/pandas/pull/39991 is merged now
updated, should fix the mypy complaint that made it into master
this now gets the code checks passing (just the bs4 thing left)
Thanks!
TYP only set default for one overloaded function in reset_index
State: closed

- [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

@simonjayhawkins I think we only need `=...` in one of the two overloads, then the error goes away and all passes:
```console
$ mypy pandas
Success: no issues found in 1244 source files
```

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

Comments:
Sorry, this is wrong:
```python
# t.py

import pandas as pd

reveal_type(pd.DataFrame([1,2,3]).reset_index(inplace=True))
```

```console
$ mypy t.py 
t.py:3: error: No overload variant of "reset_index" of "DataFrame" matches argument type "bool"  [call-overload]
t.py:3: note: Possible overload variant:
t.py:3: note:     def reset_index(self, level: Union[Hashable, Sequence[Hashable], None] = ..., drop: bool = ..., inplace: Literal[False] = ..., col_level: Hashable = ..., col_fill: Hashable = ...) -> DataFrame
t.py:3: note:     <1 more non-matching overload not shown>
t.py:3: note: Revealed type is 'Any'
```

while on master
```console
$ mypy t.py 
t.py:3: note: Revealed type is 'None'
```
[ArrayManager] Add SingleArrayManager to back a Series
State: closed

xref https://github.com/pandas-dev/pandas/issues/39146#issuecomment-786741804

This implements a `SingleArrayManager` class for backing a Series, consistent with how we have a SingleBlockManager.

cc @jreback @jbrockmendel 

Labels:
name: Refactor
description: Internal refactoring of code
name: Internals
description: Related to non-user accessible pandas implementation

Comments:
> is there a way to just have a SingleManager class that works for block / array. a SingleBlockManager after all is a single array.

I don't think so, AFAIK. A Block and an array have a quite different API (also, the SingleArrayManager is inheriting quite a bit from ArrayManager, so it's more different from SingleBlockManager than just the code added in the SingleArrayManager class here). 

There certainly might be some trivial properties that could be shared (like `.index`, and `.dtype` if if we add a `.array` to SingleBlockManager that is a shortcut to the Block's values) to reduce the size of the SingleArrayManager a bit. But note that right now it's not even 150 lines of code. 

@jreback are you OK with this approach (see my response at https://github.com/pandas-dev/pandas/pull/40152#issuecomment-789240135)
thanks @jorisvandenbossche 

ok generally ok with adding code, but more code == more complexity, so ultimately we want to simplify as much as possible. of course we are in the middle of new development on array manager so ok for now. but we want to consolidate.
PERF: extract_array -> _values
State: closed

Found that grepping for uses of pd.array is a PITA, so went through and changed them all (inside core) to pd_array.  @MarcoGorelli would it be feasible to make a code check for this?

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

Comments:
Should we then also use consistently `pd_array` in tests and remove the line

https://github.com/pandas-dev/pandas/blob/95a86a9884e6e0a32aeffff345e39434bbb722e5/scripts/check_for_inconsistent_pandas_namespace.py#L32

?  cc @jorisvandenbossche as you're commented on `pd_array` previously
> Should we then also use consistently pd_array in tests and remove the line

another option would be to define it pd_array in core.construction and import it into the pd namespace as `array`
> Should we then also use consistently pd_array in tests and remove the line

For me this PR doesn't change what I said previously about it. In tests we should prefer to use public API where possible, this PR is about internal imports.

> another option would be to define it pd_array in core.construction and import it into the pd namespace as array

That doesn't solve the issue that I want it as `pd.array` in the tests, though ;)
understood - thanks for explaining!
any non-trivial perf impact?

and should we have a rule for internal tests? (to use `pd_array`)?
>> another option would be to define it pd_array in core.construction and import it into the pd namespace as array

> That doesn't solve the issue that I want it as pd.array in the tests, though ;)

Maybe I communicated poorly. The ideas was that `pd.__init__` could have `from pandas.core.construction import pd_array as array`, so tests could still use `pd.array` or `from pandas import array`
@jbrockmendel I understood that correctly ;) (but so then it doesn't matter for tests, and since you were replying to Marco's comment about tests, I thought it was related to that)
[ArrayManager] Ensure to store datetimelike data as DatetimeArray/TimedeltaArray (and not ndarray)
State: closed

Pre-cursor for https://github.com/pandas-dev/pandas/pull/39991

Currently we didn't really check that we were consistently storing datetimelike data as the EA (DatetimeArray, TimedeltaArray) or as ndarrray. Ensuring this in the ArrayManager constructor turns up a few failures.

I *think* it will be the easiest to always store them as EA and not as ndarray (eg for many other operations, we otherwise would wrap them in the EA anyway).

Labels:
name: Refactor
description: Internal refactoring of code
name: Internals
description: Related to non-user accessible pandas implementation

Comments:
@jbrockmendel more comments here?
LGTM cc @jreback
@jbrockmendel thanks for the review

I updated https://github.com/pandas-dev/pandas/pull/39991 now based on this
REGR: reduction operations failing if `min_count` is larger
State: closed

- [ ] closes #39738
- [ ] 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: Reduction Operations
description: sum, mean, min, max, etc.

Comments:
i'll add a test and whatsnew shortly, if we want to go this route for backport purposes. This fix does not have much value in isolation as the regression https://github.com/pandas-dev/pandas/issues/39738#issuecomment-787904615 is not yet addressed here.
The regression this fixes is caused by 

```
        if np.ndim(result) == 0:
            # TODO(EA2D): special case not needed with 2D EAs
            res_values = np.array([[result]])
        else:
            res_values = result.reshape(-1, 1)
```

in pandas/core/internals/blocks.py

this does not account for the fact that `_maybe_null_out` (and therefore `nansum`) may return a scalar.

`nansum`, `_maybe_null_out` and `check_below_min_count` need some work if they are to be called with 2d arrays, but this is outside the scope of a backport PR.

I still need to track down the earlier regression, where min_count above the row/column length but below the 2d shape does not work as intended, i.e. does nothing
i don't think we should block on this for 1.2.3. This patch may involve some non-trivial work.
> i don't think we should block on this for 1.2.3. This patch may involve some non-trivial work.

sure. a 1.2.4 milestone has been created. easily moved.

> > i don't think we should block on this for 1.2.3. This patch may involve some non-trivial work.
> 
> sure. a 1.2.4 milestone has been created. easily moved.

great just don't want to block (and i don't see anything urgent to prevent release).
> and i don't see anything urgent to prevent release

I was thinking maybe wait till #40144 is fixed. also affects backport. but we will likely have another release before the np-dev issues cause mainstream issues. 
> > and i don't see anything urgent to prevent release
> 
> I was thinking maybe wait till #40144 is fixed. also affects backport. but we will likely have another release before the np-dev issues cause mainstream issues.

cool
> This fix does not have much value in isolation as the regression [#39738 (comment)](https://github.com/pandas-dev/pandas/issues/39738#issuecomment-787904615) is not yet addressed here.

this seems to be a long standing behavior. This seems non-intuitive to me since I expected min_count to apply column-wise/row-wise if axis is specified.
thanks @simonjayhawkins 

its possible we want to refactor this for 1.3, but for later, cc @jbrockmendel 
@meeseeksdev backport 1.2.x
CI, TST enable branch coverage
State: closed

- [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

As suggested on Gitter - this could help identify more untested parts of the codebase and hence uncover bugs


Labels:
name: CI
description: Continuous Integration

Comments:
What does "branch" coverage exactly mean?
@jorisvandenbossche here's an example:

```python
# t.py

def main(foo: bool):

    a = 0

    if foo:
        a += 1
    print(a)
```

```python
# test_t.py

from t import main


def test_main():
    main(True)
```

Notice how we have never tested the case where `foo` was `False`. But, if we run `pytest` with `branch=False`, it gives us 100% coverage - indeed, all lines were covered
```bash
---------- coverage: platform linux, python 3.7.10-final-0 -----------
Name    Stmts   Miss  Cover   Missing
-------------------------------------
t.py        5      0   100%
-------------------------------------
TOTAL       5      0   100%
```

However, if we run it with `branch=True`, then it picks up on the fact that we never went from line 5 directly to line 7 - we always went down the line 5->6->7 route:
```bash
---------- coverage: platform linux, python 3.7.10-final-0 -----------
Name    Stmts   Miss Branch BrPart  Cover   Missing
---------------------------------------------------
t.py        5      0      2      1    86%   5->7
---------------------------------------------------
TOTAL       5      0      2      1    86%
```

Hence, this can help identify untested parts of the codebase
Ah, thanks for the explanation!
thanks @MarcoGorelli
to_offset was not using the compiled regex
State: closed

- [x] closes #39988
- [ ] 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: Frequency
description: DateOffsets
name: Clean

Comments:
thanks @Mulugruntz
WIP: ENH: Support ZoneInfo timezones
State: closed

- [ ] closes #37654
- [ ] 1 tests added /  0 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

WIP for #37654. Starting from scratch on this one. I will mark this ready for a review when it gets the job done. Apart from refactoring the code to rely only on the public interface of tzinfo independent of the implementation, the main worry is performance. I'll make sure to benchmark appopriately.

Apologies for the long absence. Had some health issues, but they are, thankfully, pinned down now, and I have mostly recovered.

Labels:
name: Timezones
description: Timezone data dtype
name: Dependencies
description: Required and optional dependencies
name: Stale

Comments:
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.
@AlexKirko closing as stale.