BrandonFors/pandas-github-issues huggingface dataset:
[ArrayManager] TST: run all tests with separate not slow / slow build
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:
@jreback @jbrockmendel as discussed, this splits the current actions build with ArrayManager in two and runs the full test suite with slow / not slow markers.
nice.  any estimate how this affects CI runtime?
>  any estimate how this affects CI runtime?

Note that with this PR, not much changes (we were already running almost all tests in this build). It is https://github.com/pandas-dev/pandas/pull/40496 that is adding tests to all other builds
And to answer your actual question: this added one build (for the slow tests) that takes 25m.
thanks @jorisvandenbossche very nice
Two missing spaces in documentation
State: closed

I fixed the missing space that made the documentation render "isthe" instead of "is the".

Before the change:
- This **isthe** equivalent of the numpy.ndarray method argmax.
- This **isthe** equivalent of the numpy.ndarray method argmin.

After the change:
- This **is the** equivalent of the numpy.ndarray method argmax.
- This **is the** equivalent of the numpy.ndarray method argmin.


- [ ] 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:

Comments:
can you merge master?  looks like a bunch of unrelated changes crept in
@jbrockmendel
I wasn't sure what you meant. I submitted a new pull request and I am closing this one.
Thanks!
CLN: sqlalchemy deprecation, annotations, share shape properties
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: Clean
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
thanks
REF: share to_native_types with ArrayManager
State: closed

also combine implementations of Block.fill_value

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

Comments:
@jorisvandenbossche are you happy with this approach?  if so, i think can use it to get rid of at least one or two more uses of apply_with_block
> Should we try to find a better place to put the to_native_types function?

Sure.  Have somewhere in mind?
Not directly a better idea. Let's worry about that later.
TYP: index.pyi
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

Comments:
Updated+green
rebased + greenish
REF: avoid broadcasting of Series to DataFrame in ops for ArrayManager
State: closed

xref https://github.com/pandas-dev/pandas/pull/39772

Currently, we broadcast a Series to a DataFrame for elementwise operations in the "op(df, series)" case. This is done (I think) to be able to perform the operation block-wise (which is only implemented for block/block case, and not for block/array). 
But, when using the ArrayManager, this broadcasting is 1) not necessary (since we perform column-by-column anyway) and 2) costly.

Labels:
name: Refactor
description: Internal refactoring of code
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations
name: Internals
description: Related to non-user accessible pandas implementation

Comments:
BTW i think the array manager build can be simplified with something like `pytest pandas/tests/[abcdfglpstuw]*`.  Getting close! 
I am running into the following issue:

```
In [10]: np.array([1, 2]) * pd.NaT
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-10-0274fc287e43> in <module>
----> 1 np.array([1, 2]) * pd.NaT

TypeError: unsupported operand type(s) for *: 'numpy.ndarray' and 'NaTType'
```

while 

```
In [12]: np.array([1, 2]) * np.timedelta64("NaT", "ns")
Out[12]: array(['NaT', 'NaT'], dtype='timedelta64[ns]')
```

This happens if you do eg `df * ser` where the DataFrame has numerical columns, and the Series has timedelta64 dtype with NaTs. 
I suppose this will also be a problem for the untyped pd.NA

(from `pandas/tests/frame/test_arithmetic.py::TestFrameArithmetic::test_td64_op_nat_casting`)
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.
Appears this PR has been dormant for a while so closing. Feel free to reopen when you have time to work on this PR. 
@jorisvandenbossche if you can rebase
Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen.
Backport PR #40471: compat: sqlalchemy deprecations
State: closed

Backport PR #40471

Labels:
name: IO SQL
description: to_sql, read_sql, read_sql_query
name: Dependencies
description: Required and optional dependencies

Comments:
I'll merge this for now pending further investigation, the pre-commit checks passed locally before pushing this PR.

In general we don't do style backports, so we maybe need to pin versions for pre-commit on 1.2.x cc @MarcoGorelli 
> In general we don't do style backports, so we maybe need to pin versions for pre-commit on 1.2.x cc @MarcoGorelli

@MarcoGorelli I can now replicate locally after doing a pre-commit clean. Strange that yesqa is now failing when that has a pinned version.

I'll just open a PR directly against 1.2.x with the trivial changes required rather than investigating this further.
> I'll just open a PR directly against 1.2.x with the trivial changes required rather than investigating this further.

urgh. not straightforward. flake8 fails if apply the yesqa changes.
looks like adding `additional_dependencies: [flake8==3.8.4]` may fix it.
REF: prepare (upcast) scalar before dispatching to arithmetic array ops
State: closed

This moves the potential upcasting of python/numpy scalars to pandas scalars out of the actual array op  (`arithmetic_op`), but moves it up to the Series / DataFrame level before calling the array op. This avoids calling it repeatedly on the same scalar for each column.

Labels:
name: Refactor
description: Internal refactoring of code
name: Performance
description: Memory or execution speed performance
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations

Comments:
How costly is this?  I get that its a valid optimization, but it breaks the abstraction of the self-contained op.
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.
> How costly is this? 

In the same `FrameWithFrameWide` benchmark (as mentioned in https://github.com/pandas-dev/pandas/pull/39820#issuecomment-825185163), calls to `_maybe_upcast_for_op` currently take around 8% on master. But I expect this number to become (relatively) bigger with other optimizations merged (like #39820). For reference, the actual `add` operator only takes around 18% in this benchmark, with everything else being overhead that can be partly optimized away. 

> I get that its a valid optimization, but it breaks the abstraction of the self-contained op.

We already do other preparation steps before the actual array_op as well (extract_array, extracting zerodim, checking length, wrapping datetimelike, etc). So I don't think the abstraction holds right now, and there is certainly an opportunity to organize this better. For example, we can ensure to have a clear set of steps (factored out as consistent helper functions) that need to be taken in the different code paths where `array_op` gets called (eg both in series op as frame op)
> We already do other preparation steps before the actual array_op 

Fair enough.

Generally LGTM.  Should this wait until after the ensure_wrapped_if_datetimelike PR so that can be folded into this helper?
looks fine. this helps with perf?
> this helps with perf?

Yes, see https://github.com/pandas-dev/pandas/pull/40479#issuecomment-825191578
thanks @jorisvandenbossche
BUG: frame.any/all with bool_only=True and Categorical[bool]
State: closed

Started with sharing some code, unearthed a bug along the way.

Nothing user-facing since it is only in ArrayManager.

Labels:
name: Bug
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
is this user facing at all? if so, pls add a whatnew
> is this user facing at all? if so, pls add a whatnew

nothing changing for users.  only change is in ArrayManager, where the current behavior doesnt match BlockManager
looks fine, pls rebase and ping on green.
ping
TYP: get_reverse_indexer, get_group_index_sorter
State: closed

nailing down all the "np.intp"s bit by little bit.

AFAICT we're not going to be able to annotate ndarrays with ndim/dtype any time soon.  Might be worth looking into https://pypi.org/project/nptyping/

Labels:
name: Algos
description: Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
any perf changes?
> any perf changes?

might be some small gain on 32 bit, nothing on 64.
gentle ping; the intp_t stuff (this, #40475, #40474) is becoming blocker-ish for getting algos.pyi and groupby.pyi 
yeah will look a bit later
CLN: intp_t instead of int64_t for indexers in libs funcs
State: closed

The places where this will need updating proliferate, so right now im just looking to see how the 32bit builds handle this.

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

Comments:
any perf diffs?
> any perf diffs?

very little.  except for a possibly-avoided copy in Categorical, it should only show up on 32bit
can you rebase
rebased + green
CLN: factorize returns ndarray[intp], not int64
State: closed


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

Comments:
any perf changes? are the XML changes orthogonal? (not objecting)
>any perf changes?

same as the other PRs: should be neglible on 64bit, mixed on 32bit until we are internally consistent

> are the XML changes orthogonal? (not objecting)

i didnt do those, no idea how they got into the diff
looks like the XML edits are already present in master, but just rebased, so hopefully they'll dissappear from the diff
i guess general question then

i agree aligning in intp is good mainly because we need to convert when we do a take
but shouldn't we see perf gains (from not constantly converting)?
> but shouldn't we see perf gains (from not constantly converting)?

on 64bit systems intp and int64 are the same and ensure_platform_int doesnt make a copy if it doesnt have to (and is in cython, so is cheap).  on 32bit yes, we are going back-and-forth in some places but ATM it is too scattered to really track down (even if i had the hardware on hand)
TYP: tslib.pyi
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

Comments:
Thanks @jbrockmendel
compat: sqlalchemy deprecations
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: IO SQL
description: to_sql, read_sql, read_sql_query
name: Dependencies
description: Required and optional dependencies

Comments:
ok i think we should prob backport this any objections?
cc @simonjayhawkins its possible may need to backport things like this for 1.2.4
Yeah, I think this might be useful to backport
@meeseeksdev backport 1.2.x
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.2.x
$ git pull
```

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

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

```
$ git commit -am 'Backport PR #40471: compat: sqlalchemy deprecations'
```

4. Push to a named branch :

```
git push YOURFORK 1.2.x:auto-backport-of-pr-40471-on-1.2.x
```

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

> "Backport PR #40471 on branch 1.2.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).
CI: pre-commit fixups
State: closed

Fixes 2/3 of the failures on master that im aware of.  sqlalchemy is the one left out.

Labels:
name: CI
description: Continuous Integration

Comments:
need to backport these?
> need to backport these?

i think these both got introduced in the last day or so 
any experience with sqlalchemy?  ive got a branch that is most of the way there, but their deprecation message

```
The Connection.run_callable() method is deprecated and will be removed in a future release.  Use a context manager instead. (deprecated since: 1.4)
```

isnt very helpful in terms of telling me _what_ context manager to use
good to go here?  after this, #40471 should get us to green
@MarcoGorelli ok for a followup if you want to check / rename the import
BUG/TST: run and fix all arithmetic tests with+without numexpr
State: closed

Closes #40462

This PR adds a auto-used fixture for `pandas/tests/arithmetic` that sets the `expressions._MIN_ELEMENTS` to 0, to force taking the numexpr path, even if our test data are small (which would otherwise never excercise the numexpr code path)

Further, it makes some fixes to get the tests passing with this better coverage. 
The fixes are a bit quick-and-dirty, but at least already show what is needed to get things working.

Labels:
name: Bug
name: Testing
description: pandas testing functions or related to the test suite
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations

Comments:
can you merge master
> @jbrockmendel comments addressed?

Not yet
This is ready for another review now
~(although there are some failures about a wrong error message with the latest commit, I see, but aside from that should already be reviewable)~ this should be fixed, wrong bracket alignment ;)
thanks @jorisvandenbossche
TST: csv parser segfaulting on malformed input.
State: closed

xref #14782 
- [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: Testing
description: pandas testing functions or related to the test suite
name: IO CSV
description: read_csv, to_csv

Comments:
cool, failures addressed elsewhere, merge on greenish
thanks @ahawryluk
REF: back DatetimeBlock, TimedeltaBlock by DTA/TDA
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

Broken off from #40149.   Like #40149, this backs DTBlock/TDBlock by (sometimes 2D) DTA/TDA.  Unlike #40149, this does _not_ make the DTA backing DatetimeTZBlock sometimes-2D.

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

Comments:
rebased + green.  between this and #40477 we're within striking distance of ripping out a couple more Block subclasses
> just a question. what's failing on master?

havent looked this morning, but last night it was something td64/dt64-casting related merged into npdev
TYP: stubs for reshape, ops, ops_dispatch, hashing
State: closed

manually curated, extra specificity in comments it is supported directly

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

Comments:
> manually curated, extra specificity in comments it is supported directly

IIUC The pyright stubs are mainly for the public api. for our internal consistency checking , pandas._libs includes functions that we use internally, so manually curated is probably best for now.
Will update per suggestions.

> IIUC The pyright stubs are mainly for the public api. for our internal consistency checking , pandas._libs includes functions that we use internally, so manually curated is probably best for now.

Following the dev call last month (two months ago?) about stubs I looked over pyright's stubs for _libs and found them not-totally-accurate (mostly Timestamp/Timedelta IIRC), so I plan to do all of these manually.
Thanks @jbrockmendel
BUG/API: Index.__new__ match Series datetime inference behavior
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

This makes us slightly more aggressive in casting to dt64 in `Index.__new__` (see the added test_index_new test, it contains all of the cases that are reached in the existing tests where the current behavior differs from the PR behavior)

Technically an API change, but I think its worth it to match the Series behavior.

Found+fixed a `pd.array` bug along the way.

Labels:
name: Index
description: Related to the Index class or subclasses
name: API - Consistency
description: Internal Consistency of API/Behavior

Comments:
@MarcoGorelli any idea whats going on with the docstring validation script test?
mothballing to clear the queue
Fix StringArray.astype for category dtype
State: closed

- [X] closes #40351 
- [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

This was failing due to `elif np.issubdtype(dtype, np.floating)` [here](https://github.com/pandas-dev/pandas/blob/master/pandas/core/arrays/string_.py#L330), which fails for the pandas dtypes.
StringArrays are now being cast to Categorical just as they were before https://github.com/pandas-dev/pandas/pull/38530, when the casting still happened inside `Block`.

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

Comments:
This uncovered a bit of a larger issue, basically #37626, but for datetimes & period.

Example: 
```python
import pandas as pd
t = pd.Series(["1/1/2021", "2/1/2021", None], dtype="period[M]")
s = pd.Series(["1/1/2021", "2/1/2021", None], dtype="string").astype("period[M]")
```

`t` work, `s` raises because the `None` is converted to `NA` which isn't recognized as datetimelike by period.

Example
```python
import pandas as pd
t = pd.Series(["1/1/2021", "2/1/2021"], dtype="object").astype("datetime64[ns]")
s = pd.Series(["1/1/2021", "2/1/2021"], dtype="string").astype("datetime64[ns]")
```
`t` work, `s` raises.

However if it's ok I'd open a new issue for it, since it'll be a larger revamp which will take me a few more days.
These "new" issues also aren't regressions like this Categorical issue, they're all already present in 1.2.3.
Yes, that's a more general issue, and can be left out of scope for this PR.
> needs a whatsnew note for 1.2.4

This doesn't need a whatsnew, it's only a regression on master
> > needs a whatsnew note for 1.2.4
> 
> 
> 
> This doesn't need a whatsnew, it's only a regression on master

ok great
Why does Codecov say 0% coverage for the diff, when the added `if`-branch has multiple tests running over it?
@jreback added a Whatsnew, should be good for merge.
thanks @siboehm
PERF: no need to check for DataFrame in pandas.core.computation.expressions
State: closed

In practice, this case is only covered by specific tests using DataFrame in `test_expressions.py`, not from actual usage through ops (so I just updated the failing tests to not use any DataFrame as input to those checks). 
Although our test suite might not be the best suited to check that, since many tests are small and thus don't use the numexpr path... (fixing that in https://github.com/pandas-dev/pandas/pull/40463)

Labels:
name: Performance
description: Memory or execution speed performance
name: Algos
description: Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff

Comments:
cc @jreback you might be more familiar with the history behind this. I suppose at some point, DataFrames were passed directly to `evaluate` in `expressions.py`. But now we are only passing arrays from `array_ops.py`, so I just updated the tests.
@jreback OK here?
thanks @jorisvandenbossche
[ArrayManager] DataFrame constructor from ndarray
State: closed

xref https://github.com/pandas-dev/pandas/issues/39146

Currently, the `DataFrame(ndarray)` construction for ArrayManager still went through BlockManager and then `mgr_to_mgr` conversion, which is less efficient. 
This PR adds a check in `ndarray_to_mgr` for directly creating an ArrayManager.

~It's still WIP, because there are still some validation steps that are now done inside BlockManager/Block inits, that need to be factored out / shared / added to ArrayManager path.~

Labels:
name: Refactor
description: Internal refactoring of code
name: Internals
description: Related to non-user accessible pandas implementation
name: Constructors
description: Series/DataFrame/Index/pd.array Constructors

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.
ENH: `escape` html argument in Styler.format
State: closed

- [x] closes #40388 

Adds an HTML `escape` parameter to `Styler.format`, which acts to escape the data value **before** it is passed to a formatter.

For example, `Styler.format('<a href="a.com/{0}">{0}</a>', escape=True)` will not escape the HTML as part of the formatting function, it will only escape the value `{0}` going into the formatter.

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

Comments:
cc @jnothman if comments.
also checks are failing
> also checks are failing

when fixing these I noticed that the `na_rep` was not working with `escape` since missing values were escaped before being replaced.

so I refactored to fix this and added a test for it.

@jreback ping is green.
this should be good to go in now, I believe.
thanks @attack68
TYP: stubs for tslibs
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

Comments:
in setup.cfg we have `ignore_missing_imports = True`. This is probably the next strictness flag we should consider changing.

we this PR have 393 errors, 162 of these are from pandas._libs

on master we have 407 errors (176 pandas._libs)

so a good start.
Thanks @jbrockmendel