BrandonFors/pandas-github-issues huggingface dataset:
Backport PR #38120: API: preserve freq in DTI/TDI.factorize
State: closed

Backport PR #38120

Labels:
name: Frequency
description: DateOffsets
name: MultiIndex

Comments:
@jbrockmendel if you can give this a quick eyeball before I merge. several merge conflicts where I could have gone wrong. see also comments in #38137 
taking a look now
The PeriodArray thing looks unnecessary but harmless, otherwise LGTM
> The PeriodArray thing looks unnecessary but harmless, otherwise LGTM

see https://github.com/pandas-dev/pandas/pull/38137#issuecomment-735395004
makes sense, thanks
Backport PR #38148: ENH: Improve performance for df.__setitem__ with list-like indexers
State: closed

Backport PR #38148

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

Comments:
@phofl https://github.com/pandas-dev/pandas/pull/38148#discussion_r532229038 was this necessary? do we know what commit caused the performance regression. the existing code on 1.1.x looks much simpler to what was replaced on master.
The underlying performance problems existed with this solution too. The loop is the actual problem, not the if else logic.
That is tricky. The tests are failing, because ``Index.union`` is different on master.

```
idx = pd.RangeIndex(0, 2)
result = idx.union([1])
```

This returns a ``RangeIndex`` on master and an ``Index`` with ``dtype="object"`` on 1.1.4. 

Suggestions about how to proceed here?

cc @simonjayhawkins 
Since this was not a performance *regression* (at least, the issue https://github.com/pandas-dev/pandas/issues/37954 does not mention anything about a regression, AFAIK), I don't think we should be backporting this.
> Since this was not a performance _regression_ (at least, the issue #37954 does not mention anything about a regression, AFAIK), I don't think we should be backporting this.

I agree.

closing this an can maybe revisit pending outcome of discussions in #38148

if not reverted the release note needs to be moved, but this is tracked by https://github.com/simonjayhawkins/pandas-release/actions/runs/391543194 so won't be missed.
Rewrite dict literal for files in tests/reshape/
State: closed

- [ ] xref #38138 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Rewrites as dictionary literals for the following files:
 - ./pandas/tests/reshape/concat/test_datetimes.py
 -  ./pandas/tests/reshape/merge/test_merge.py
 -  ./pandas/tests/reshape/merge/test_multi.py

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

Comments:
thanks @skvrahul
PERF: Introducing hash tables for complex64 and complex128
State: closed

- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Next step for #33287

The complex case is somewhat less straight forward, because Cython [defines](https://github.com/cython/cython/blob/master/Cython/Includes/numpy/__init__.pxd#L755) `complex128`/`complex64` as:

```
ctypedef float complex  complex64_t
ctypedef double complex complex128_t
```

where `double complex` actually means a Cython-datatype defined here https://github.com/cython/cython/blob/master/Cython/Utility/Complex.c#L56

Thus a conversion between numpy's complex and Cython's complex must happens somewhere.





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

Comments:
FYI @jbrockmendel 

Some polishing is still needed and probably additional testing, but this is how it could look like.

Maybe you know a better solution for the whole numpy's <->cython's complex-business.

There are also some details, which maybe need some tinkering:
   * Right now,  `nan+1j` and `1+nan*j` are considered not-a-number. However, they aren't considered equivalent. Not sure that doesn't make any problems.
   * `inf +nan*j` is considered not-a-number, maybe it makes more sense to see it as infinity and not not-a-number


Failures seem to be unrelated to my changes...
> Failures seem to be unrelated to my changes...

Yes.  The npdev failure I think should be fixed by #38209 and the other one just annoyingly happens sometimes because hypothesis is slow.  Don't worry about either.
can you merge master and will look again
ok looks good. this complex 64/128 are available on all platforms?
> ok looks good. this complex 64/128 are available on all platforms?

@jreback It should work on platforms which have 64bit- and 32bit floating numbers. As we don't really use complex arithmetic it is not important whether C-compiler supports complex-numbers or not.
thanks @realead
BUG: preserve nullable dtype for float result in IntegerArray/BooleanArray arithmetic ops
State: closed

xref https://github.com/pandas-dev/pandas/issues/38110

This ensures that an operation on IntegerArray that results in floats (eg division) returns a FloatingArray instead of numpy float array.

This is only a small change in the actual code, the bulk of the changes is updating the tests. 

Labels:
name: NA - MaskedArrays
description: Related to pd.NA and nullable extension arrays

Comments:
looks ok, needs a whatsnew note 
> needs a whatsnew note

Added a sentence to the section we already have about the new nullable floating dtype
Failures are the ones present on master as well
Revert "REF: define nanargminmax without values_for_argsort"
State: closed

Reverts pandas-dev/pandas#37815

Labels:
name: Algos
description: Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
See discussion on the PR https://github.com/pandas-dev/pandas/pull/37815, argmin/argmax on purpose didn't do a full argsort for performance reasons
I will do a PR updating the docs / comments on expectations how EA authors should implement this in a follow-up
Failure are the npdev failures present on master as well
The error message for unsupported index on shift() could be more explicit
State: closed

When the type of data index is not datetime-like, the following error message will raise:
`NotImplementedError: Not supported for type Index`
 
The error message could be more useful by explicitly stating which types of data that `shift` supports.
 
Expected error message:
` f"This method is only implemented for DatetimeIndex, PeriodIndex and TimedeltaIndex; Got type {type(self).__name__}."`


Labels:
name: Error Reporting
description: Incorrect or improved errors from pandas

Comments:
Hello @ZhihuiChen0903! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:







There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-12-03 14:19:42 UTC
lgtm @ZhihuiChen0903 if you turnt his off draft can merge.
@jreback I have turned off draft.
thanks @ZhihuiChen0903
ENH: support nanoseconds in Period constructor
State: closed

- [x] closes #34621
- [x] closes #17053
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Picking up #34720

ASVs:
```
asv continuous -f 1.1 upstream/master HEAD -b ^period

       before           after         ratio
     [3f510600]       [2b2f1d04]
     <GH34621-nanos-for-Period^2>       <GH34621-nanos-for-Period>
+      7.46±0.2ms       11.6±0.1ms     1.55  period.PeriodIndexConstructor.time_from_ints('D', False)
+      7.57±0.3ms      11.4±0.03ms     1.51  period.PeriodIndexConstructor.time_from_ints('D', True)
+      52.8±0.6ms       59.2±0.4ms     1.12  period.PeriodIndexConstructor.time_from_ints_daily('D', True)
+      52.9±0.9ms       59.0±0.1ms     1.11  period.PeriodIndexConstructor.time_from_ints_daily('D', False)
-      3.21±0.2μs      2.90±0.02μs     0.90  period.Indexing.time_get_loc

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE DECREASED. 
```

cc @jreback https://github.com/pandas-dev/pandas/pull/34720#discussion_r491536763 in case we want this for 1.2

Labels:
name: Period
description: Period data type

Comments:
thanks @arw2019
BUG: array-like quantile fails on column groupby
State: closed

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


Labels:
name: Bug
name: Groupby

Comments:
Hello @GYHHAHA! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-12-02 12:33:48 UTC
thanks @GYHHAHA very nice
CI: update tests for numpy 1.20 change to floordiv
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Looks like https://github.com/numpy/numpy/pull/16161 changed numpy's floordiv behavior

Labels:
name: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
Yikes, I got this passing, but i have no earthly idea what is going on in the Sparse tests
can we push an xfail for this case for now? (and debug / followup with this PR later).
failing on 1.1.x as well, so will want to try to backport this.
agreed with the above
let's just xfail for now and backport
updated to xfail
narrowed down the xfails a little bit
thanks @jbrockmendel  (feel free to open an issue to address this, no hurry now).
@meeseeksdev backport 1.1.x
TST: add comment that plotting tests are slow
State: closed

- [ ] xref https://github.com/pandas-dev/pandas/pull/38079#issuecomment-735262800
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Add comment (module docstring) in ``pandas/tests/plotting/common.py``,
mentioning that all plotting tests are marked as slow.

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

Comments:
thanks @ivanovmg
BUG: unexpected merge_ordered results caused by wrongly groupby
State: closed

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


Labels:
name: Bug
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode

Comments:
Hello @GYHHAHA! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-11-30 03:51:58 UTC
thanks @GYHHAHA very nice
fix frame, generic, series.py
State: closed

- [x] xref #38138
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry
        Identify unnecessary dict call - rewrite as a literal


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

Comments:
thanks @liudj2008 
Thank you @ivanovmg and @jreback . Will look more issues with pandas!
BUG/CLN: Minimize number of ResourceWarnings
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

I think I got all `ResourceWarning`s when running:
`PYTHONWARNINGS=default pytest pandas/tests/io`

Labels:
name: IO Data
description: IO issues that don't fit into a more specific label

Comments:
thanks @twoertwein
DOC: Add examples for filling missing data
State: closed

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


Labels:
name: Docs
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: Stale

Comments:
Hi @chrystalchan-academic - is this still active?
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.
closing for now, let us know if you'd like to continue working on this and we'll reopen
BUG: assert_frame_equal exception for datetime #37609
State: closed

closes #37609 

- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Working with @richardwong8 on this issue.

This addresses issue #37609 where a check `needs_i8_conversion(left.dtype) or needs_i8_conversion(right.dtype)` in assert_series_equal caused an assert_extension_array on left._values and right._values when DatetimeArray or TimedeltaArray were used. This caused an exception to be raised when either the right or left was not an instance of ExtensionArray but was used with DatetimeArray or TimedeltaArray. 

This would have been covered in the earlier check `is_extension_array_dtype(left.dtype) and is_extension_array_dtype(right.dtype)`, however, DatetimeArray and TimedeltaArray are still experimental and their dtypes are not instances of ExtensionDtype subclass.

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
also pls add a whatsnew note for 1.2, bug fixes other is fine
thanks @ssortman
CLN: improve is_unique check in Index.intersection
State: closed

- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

```
       before           after         ratio
     [e99e5ab3]       [79d6a50f]
     <cln_intersection~1>       <cln_intersection>
-        918±20ns          819±2ns     0.89  index_object.Range.time_min_trivial
-         465±7ns          414±7ns     0.89  index_object.Range.time_get_loc_dec
-         464±6ns          411±7ns     0.89  index_object.Range.time_get_loc_inc
-      16.6±0.7ms      12.6±0.03ms     0.76  index_object.SetOperations.time_operation('date_string', 'union')
```

Seems to have no impact.

cc @jreback


Labels:
name: MultiIndex
name: Clean

Comments:
ok let's merge this for 1.2, thought it would have more of an impact.
moving this off 1.2 for a refactor
Added the unique check to Index.unique. Categeorical.unique does remove values which are not within the categories. If we want to keep this, we can not add a check for uniqueness
can you run relevant benchmarks and see if this makes any difference?
asvs in index_object:

```

       before           after         ratio
     [e99e5ab3]       [aba0f2a3]
     <master>         <cln_intersection>
-        47.6±1ms       41.4±0.2ms     0.87  index_object.SetOperations.time_operation('strings', 'intersection')
-      17.0±0.2ms       13.7±0.5ms     0.80  index_object.SetOperations.time_operation('date_string', 'union')
```
LGTM
thanks @phofl
BUG: loc raising KeyError for string slices in list-like indexer and DatetimeIndex
State: closed

- [x] closes #27180
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

I looked into this and added an implementation for list-like indexers containing slices and DatetimeIndexes. If we want to support this we probably have to do something like this.

Labels:
name: Datetime
description: Datetime data dtype
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Needs Discussion
description: Requires discussion from core team before further action

Comments:
There are a couple of issues here, first of which is that it isn't obvious that we actually should support this.  It does improve internal consistency, but adds a lot of complexity.

Even if we do decide to move forward with this, id want to start with making get_indexer behave like a vectorized get_loc.

(ideally id like to get rid of of the `FooIndex._convert_bar_indexer` methods altogether)
> There are a couple of issues here, first of which is that it isn't obvious that we actually should support this. It does improve internal consistency, but adds a lot of complexity.
> 
> Even if we do decide to move forward with this, id want to start with making get_indexer behave like a vectorized get_loc.
> 
> (ideally id like to get rid of of the `FooIndex._convert_bar_indexer` methods altogether)

+1 here. yeah partial date indexing has only very limited support now, we don't wish to expand this, its mainly useful for interactive use. 

unless other objections going to close the original issue.

thanks @phofl for the effort here.
No worries, I was not sure either if this would be desirable. But wanted to share a possible solution before discussing this. 
did we settle on closing this?
Think so?
closing, though we should have a test and good error message if someone attempst this. @phofl want to repurpose this?
Yep will do 
Currently 

```
index = pd.date_range('2001-01-01', periods=100)


s = pd.Series(1, index=index)
s.loc[['2001-01']]
```

returns
```
2001-01-01    1
dtype: int64
```

which is obviously wrong. I would change this to raise if this is attempted.
> `s.loc[['2001-01']]`

I'm not sure how avoidable this is given that `Timestamp("2001-01")` returns 2001-01-01, but if you've got an improvement in mind, go for it
Yep, you are probably right. Have not found a way to avoid this. Closing this pr.
CLN: remove unused coerce arg in NDFrame._convert
State: closed

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


Labels:
name: Refactor
description: Internal refactoring of code
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
lgtm ping on greenish
ping
BUG: loc dropping levels when df has only one row
State: closed

- [x] closes #10521
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Originally this case was dispatched to xs, which dispatched to ``_get_loc_level``, which raised if a slice is not ``slice(None, None)`` and went back to our original function. If the ``DataFrame`` contains only one row, ``_get_loc_level`` does not raise and hence we drop levels. 

I added len(self.obj) for performance reasons becasue I am not sure if the any check for slices is faster than supressing the IndexingError in general.

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

Comments:
>  If the DataFrame contains only one row, _get_loc_level does not raise and hence we drop levels.

Should the _get_loc_level behavior correct?
From reading the code I think this is the intended behavior, but not quite sure.
It looks like jreback and I had different suggestions, but the theme to both is "can this be handled any earlier?"  have you ruled out the lower-level options?
Sorry tough week at work. We can not call xs, because this would return wrong results. Do you mean mit earlier before reaching this point? Because I understood the suggestions as doing this deeper down in the calling stack
> Do you mean mit earlier before reaching this point? Because I understood the suggestions as doing this deeper down in the calling stack

Deeper in the call stack is ideal, yes.
@phofl are you of the opinion this is the best available approach?  you're pretty well versed in this part of the code and im inclined to trust your opinion.
Forgot that one unfortunately, had 3 pretty busy weeks at work, but will be better now.

I am not 100% sure, but I tried this out a bit locally. We have to do this if condition somewhere before dispatching to xs, ``_get_label`` itself is not really an option, would be too ugly. Doing it in ``_handle_lowerdim_multi_index_axis0`` is possible, but would be quite ugly, because the if condition is the same case as raising ``TypeError`` or ``InvalidIndexError``. This would be quite ugly, also I think this is just unnecessary convolution for the other case where ``_handle_lowerdim_multi_index_axis0`` is called.
Was a bit hasty. The problem only occurs for Objects with only one row and slices in indexer. Objects with more than one row have to go in there, no matter if the indexer has slices.
> Was a bit hasty. The problem only occurs for Objects with only one row and slices in indexer. Objects with more than one row have to go in there, no matter if the indexer has slices.

this is very strange then, these must be hitting a different path that other types, no? maybe something different in the indexing engine itself? its pretty odd to special case like this.
Looked into this again, the issue is with Series, the dimension of the MultiIndex should be reduced there, #6022
cc @jbrockmendel if any commetns.
thanks @phofl
ENH: add "schema" kwarg to io.sql.get_schema method
State: closed

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

Labels:
name: IO SQL
description: to_sql, read_sql, read_sql_query

Comments:
lgtm. @jorisvandenbossche if you can give a once over
cc @jorisvandenbossche 
lgtm ping on green
Green
Thanks @arw2019
BUG: combine_first does not retain dtypes with Timestamp DataFrames
State: closed

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

Labels:
name: Bug
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
Why does combin_first use expressions.where instead of mgr.where? Casting should be handled correctly there
> Why does combin_first use expressions.where instead of mgr.where? Casting should be handled correctly there

That line hasn't been touched since #17744, will look
Now that i have an actual keyboard: this would likely also require fixing Block.where #38073
> Now that i have an actual keyboard: this would likely also require fixing Block.where #38073 

Ok - in this case this would sit on top of that

I'm struggling a little with the pattern. Atm I have:
``` python
    def combine_first(self, other: DataFrame) -> DataFrame:
        def combiner(x, y):
            mask = extract_array(isna(x))

            x_values = extract_array(x, extract_numpy=True)
            y_values = extract_array(y, extract_numpy=True)

            # If the column y in other DataFrame is not in first DataFrame,
            # just return y_values.
            if y.name not in self.columns:
                return y_values

            _where = self._mgr.where(y_values, mask, align=True, errors="raise", try_cast=True, axis=1)
            # extract array from _where

        return self.combine(other, combiner, overwrite=False)
```
Is this in the right ballpark? 
> is the current patch not good for 1.2? (agreed could use some refactoring).

This looks good for now
@arw2019 ok small test comment. ping on green.
thanks @arw2019 
@arw2019 do you have time to try to make this go through `self._mgr.where`?  (no hurry)
> @arw2019 do you have time to try to make this go through `self._mgr.where`? (no hurry)

yes, in the works