BrandonFors/pandas-github-issues huggingface dataset:
BUG: passing TDA and wrong freq to TimedeltaIndex
State: closed

Caught while working towards making TimedeltaArray._simple_new more strict about its inputs.

- [ ] closes #xxxx
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Timedelta
description: Timedelta data type
name: Frequency
description: DateOffsets

Comments:
> Are there perf implications of going through _from_sequence instead of _shallow_copy / _simple_new ?

Yes, this removes two fast-paths.  The alternative is adding the relevant checks to those fastpaths, but I prefer to avoid the duplication that entails.



```
In [3]: tdi = pd.timedelta_range("1 day", periods=10000)                                                                                                                                                                                                
In [4]: tda = tdi._data                                                                                                                                                                                                                                 
In [5]: %timeit pd.TimedeltaIndex(tdi)                                                                                                                                                                                                                  
7.48 µs ± 261 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)   # <-- master
20.6 µs ± 210 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)   # <-- PR

In [6]: %timeit pd.TimedeltaIndex(tda)                                                                                                                                                                                                                  
4.93 µs ± 49.5 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)   # <-- master
17.4 µs ± 317 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)   # <-- PR
```

Once this bug is fixed, we'll be able to simplify TDA/TDI._simple_new which should make up some lost ground.

Also there's an easy optimization to make in sequence_to_td64ns that brings the times here to 15.5us and 11.9us, respectively.
I guess we could just add `and freq is None` to the TDA check and keep the fastpaths.
> I guess we could just add and freq is None to the TDA check and keep the fastpaths.

That sounds good to me as well. In any case, I would find it a bit strange that passing an TimedeltaArray to TimedeltaIndex(..) wouldn't be a relatively fast path.
updated to restore fastpath, just skips it if freq is non-None
thanks, if we don't have already, some asvs on Index constructions from equiv Arrays would be good.
PLT: Color attributes of medianprops etc are lost in df.boxplot and df.plot.boxplot
State: closed

- [x] closes #30346 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Found out that not only `medianprops` info is lost, so is `whiskerprops`, `capprops` and `boxprops`, so I also added them alongside. Since this is a visualization PR, so add a screenshot to show what it looks like after the change

<img width="587" alt="Screen Shot 2020-01-24 at 4 16 26 PM" src="https://user-images.githubusercontent.com/9269816/73079678-eb468180-3ec4-11ea-9628-5ab13e73f15c.png">


Labels:
name: Visualization
description: plotting

Comments:
gentle ping @TomAugspurger @jreback for feedback
Thanks @charlesdong1991 
This also closes #33868.
(see SO answer https://stackoverflow.com/a/62718470/3944322)
DOC fix truncated documentation for Series.unstack(), Series. 
State: closed

fixes https://github.com/pandas-dev/pandas/issues/31235

Changed "e.g. " to "e.g., "
Changed "a.k.a. " to "also known as"
This helps avoid the truncation caused in the docstring

Labels:
name: Docs

Comments:
No strong objection but I do think this is a little hacky. Do you know where this limitation is being imposed?
The issue here is that, every time a ". " (full stop + space) is encountered, it's presumed to be the end of a one line docstring function description.
Thanks @anjalis2112
BUG: DatetimeIndex.get_loc/get_value raise InvalidIndexError
State: closed

almost done with the get_value/get_loc methods

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

Comments:
needs a rebase. not user facing right?
> not user facing right?

technically get_loc is part of the Index api
ok, though I feel this needs more tests, but maybe just being paranoid
> ok, though I feel this needs more tests

yah, i spent most of yesterday trying to organize/flesh-out the index/indexing tests and it turned in to a yak-shaving kind of day.  this is gonna be a marathon.  xref #31241 #31193.
Update algorithms.py
State: closed

Pass the dtype to `_from_sequence` in `_reconstruct_data` in order to support ExtensionArrays with ExtensionDtypes that have parameters.

- [ ] closes #31252 
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:

Comments:
seems fine, needs tests
Slight error on my part- I hadn't realized that the `values` parameter in `_reconstruct_data` is not actually an ndarray, as the docstring notes.  In this case it is the ExtensionArray type. My code was checking for an empty array before checking to see if it was the same type as the class.  I can simply return the empty `values` GenotypeArray.

That said, I think this change couldn't hurt- it's just passing an optional parameter that already exists.  

I discovered this while utilizing the built-in tests.  It looks like the `base.BaseMethodsTests` covers this already. 
@jrm5100 if this doesn't change anything then (test or user xp), then I don't think we need to change. do you have a failing example that this patches?
For now it looks like this function (`_reconstruct_data`) is called in a few algorithms, but `factorize` seems to be the only relevant one (where an `ExtensionArray` is passed to this function explicitly.

- `unique`, where `ExtensionArray` are dispatched to their own method
- `factorize`, where I ran into this issue.
- `_value_counts_arraylike`, which doesn't handle `ExtensionArray` differently
- `mode`, which has a special case for `Categorical` but not `ExtensionArray`

For `factorize`, the `dtype` is specifically saved to a variable and passed to the function:

```python
    if is_extension_array_dtype(values):
        values = extract_array(values)
        codes, uniques = values.factorize(na_sentinel=na_sentinel)
        dtype = original.dtype
#... 
uniques = _reconstruct_data(uniques, dtype, original)
```

`_reconstruct_data` calls the `_from_sequence` of the dtype's associated `ExtensionArray` which optionally takes this parameter:

```python
@classmethod
def _from_sequence(cls, scalars, dtype=None, copy=False):
```

I'll admit that I don't have an example explicitly requiring this change.  I do think passing an optional parameter instead of ignoring it (when it is readily available) makes sense.  I also think this is probably something that could come up again in the future if this function is used with `ExtensionArray` elsewhere.

I understand if you view it as unnecessary.  I do think the minor docstring fix should be made- would have saved me and possibly others a bit of time when implementing an ExtensionArray.
> I'll admit that I don't have an example explicitly requiring this change. I do think passing an optional parameter instead of ignoring it (when it is readily available) makes sense.

You're right that this makes sense, but as a practical matter, no one is going to merge this if it doesn't have tests.
closing, if you can find a case that hits this we can reopen
TST: Fix timestamp comparison test
State: closed

- [x] closes #31175
- [x] tests added / passed
    - tests/scalar/timestamp/test_timestamp.py
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry
    - Not needed for fixing a test


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

Comments:
Thanks! @jreback think worth backporting?
sure, thanks @Dr-Irv
ENH: Implement DataFrame.value_counts
State: closed

- [x] closes #5377
- [x] tests added / passed
- [x] passes `black pandas`
- [x] whatsnew entry

This is picking up where https://github.com/pandas-dev/pandas/pull/27350 left off because I think it'd be a nice feature to have. At least one thing that still needs to be done is implementing `bins` when we have only a single column in `subset`, in which case maybe we can just delegate to `Series.value_counts`.

Labels:
name: Enhancement
name: DataFrame
description: DataFrame data structure

Comments:
also pls merge master
Getting some seemingly unrelated test failures from `test_value_counts_unique_nunique_null` in `pandas/tests/base/test_ops.py` after merging master (I don't _think_ any of this PR touches code tested there since the `Series` method is staying the same). Could this be related to the refactoring from https://github.com/pandas-dev/pandas/pull/32046?
@jreback Green after the CI fixes, thanks for the review
thanks @dsaxton 

very nice!
COMPAT: tzawareness behavior to be same as datetime
State: closed

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


Labels:

Comments:
cc @jbrockmendel 
BTW, i think once this is done we can get rid of some (all?) uses of comp_method_OBJECT_ARRAY
@MomIsBestFriend still active? Can you try to get CI passing?
@WillAyd I don't know if this is still active, I am not totally familiar with the code base, and this might been not the right issue to take, might be out of my level, I think I'll close this, due to the fact I can't really solve it. sorry for the waste of time.
REGR: Prevent indexes that aren't directly backed by numpy from entering libreduction code paths
State: closed

- [X] closes #31223
- [X] closes #31248 
- [X] tests added / passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

No whatsnew since this is a regression.

Labels:
name: Groupby
name: Regression
description: Functionality that used to work in a prior pandas version
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
Okay, I've added back `Index._has_complex_internals` but modified it to also be `True` for `CategoricalIndex`, `IntervalIndex`, and `PeriodIndex` since those indexes require (potentially expensive) conversion to get a numpy array of values.  I don't want to exclude all extension indexes since `DatetimeIndex` and `TimedeltaIndex` don't require expensive conversion.

If adding back `Index._has_complex_internals` is acceptable, then I think we can also use it fix #31248 by using it to exclude things from `libreduction` in a similar manner.  Was planning to do it in a follow-up but can do it here too if we want.
@jreback I think this is fixing a regression. We should leave it on the 1.0.0 milestone.
Merged master to fix the CI failure.
Added an additional check against `_has_complex_internals` that fixes #31248 and the associated test cases.

I don't immediately see any other areas where this check needs to occur but it's possible that there are additional cases.
thanks
Owee, I'm MrMeeseeks, Look at me.

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

1. Checkout backport branch and update it.

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

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

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

```
$ git commit -am "Backport PR #31238: REGR: Prevent indexes that aren't directly backed by numpy from entering libreduction code paths"
```

4. Push to a named branch :

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

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

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

And apply the correct labels and milestones.

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

If these instruction are inaccurate, feel free to [suggest an improvement](https://github.com/MeeseeksBox/MeeseeksDev).
                
@jschendel can you push a backport for this, didn't automatically work.
CI: Fix Assign CI not working with quotes
State: closed

- [x] closes #29768 
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Examples(all three passed)
![image](https://user-images.githubusercontent.com/47963215/72952081-a6481100-3d45-11ea-8845-338f2a2f17a4.png)



Labels:

Comments:
Thanks for working on this. Do you know why this works? I'm wondering if it's because the content of the variable is assigned and taking a substring is having no effect. 
@datapythonista I think I got it now. 
Can you take another look? Thanks. 
Thanks @lithomas1, I think this fix is better than what I proposed.

For the next time, I recommend you use a hit branch, and not work directly in master.

(Just trying " this, to see if it works as expected).
WIP/TST: add dt64tz to indices fixture
State: closed

@TomAugspurger @jorisvandenbossche as you can see, the `__array_ufunc__` that this implements is an unholy mess.  This all came from a much smaller goal: I wanted to add the "datetime-tz" key in tests.indexes.conftest.

That broke a test for `np.isfinite`, so I started implementing `array_ufunc`.  Then that broke other ufuncs, and so on and so on.

I guess I could just xfail the one isfinite test.  In retrospect that would have saved a lot of effort.  Woops.

Anyhow, we probably want/need to implement this eventually anyway, so any ideas on how to do it but less awful?

xref #31219.

Labels:
name: Mothballed
description: Temporarily-closed PR the author plans to return to

Comments:
Closing in favor of #31241
REGR: Fix IntervalIndex.map when result is object dtype
State: closed

- [X] closes #31202
- [X] tests added / passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

No whatnew since this is a regression.


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

Comments:
thanks @jschendel
(Fixes CI) Replaced set comprehension with a generator
State: closed

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


Labels:
name: CI
description: Continuous Integration

Comments:
ping @WillAyd 
Thanks!
TST: parametrize tests
State: closed

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


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

Comments:
thanks @MomIsBestFriend
WIP: TST: pass random names in tm.makeDateIndex
State: closed

@WillAyd nearly all of the remaining test failures locally are in the json tests.  can you take a look and suggest whether we just need to pass `check_names=False`

I expect if we do something like this we won't want to actually randomize; that part I guess is a proof of concept for now.

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Mothballed
description: Temporarily-closed PR the author plans to return to

Comments:
Just at a brief glance not all of the JSON formats preserve index names. Split is a particular example which came up in #17393
Just curious: what was the motivation for adding this?
> Just curious: what was the motivation for adding this?

We are using these functions for fixtures that are aimed at trying to cover a wide swath of possible index objects, but are missing large parts of that space.
closing to clear the queue, will keep trimming away at these locally
REF/TST: collect misplaced tests
State: closed

ATM we are mixing/matching pytest vs UnitTest idioms, which is confusing for me because I have to track down both inheritance and fixtures.  So I'm trying to clean this up, leading to some yak shaving.


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

Comments:
lint check is complaining about a file that doesnt appear to exist locally
thanks
Backport PR #31215 on branch 1.0.x (Follow-up: XLSB Support)
State: closed

Backport PR #31215: Follow-up: XLSB Support

Labels:
name: Docs
name: IO Excel
description: read_excel, to_excel

Comments:
dev failures should be fixed by https://github.com/pandas-dev/pandas/pull/31209. The merge commit should pass.
(wip) BUG: replacing one column's values was changing other columns' dtypes
State: closed

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


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

Comments:
I think the flattening can be done later in the code, will update this later

EDIT
----
update done
Actually, I think we can still do better - will update it later this week

EDIT
----
done
Follow-up: XLSB Support
State: closed

Updated min version for `pyxlsb` to 1.0.6 and removed the now unnecessary xfail for read from url. Turns out the other xfail is actually unnecessary so I've removed it.

Question: is my name supposed to show up in the contributors list?

- [X] closes #8540
- [X] tests added / passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry

Labels:
name: Docs
name: IO Excel
description: read_excel, to_excel

Comments:
@Rik-de-Kort yea your name should auto-generate as a contributor when we release

Thanks!
> @Rik-de-Kort yea your name should auto-generate as a contributor when we release
> 
> Thanks!

Excellent, and you're welcome. Maybe next patch we go a bit quicker, eh? :P
> @Rik-de-Kort yea your name should auto-generate as a contributor when we release
> 
> Thanks!

@WillAyd Happy to see the release is out, with my code :D However, my name does not appear to have been auto-generated in the contributors list. I do have the tag here though. Any ideas?
Hmm @TomAugspurger any ideas on the contributor piece? Original PR of #29836 was part of 1.0
BUG: Fixed upcast dtype for datetime64 in merge
State: closed

Closes https://github.com/pandas-dev/pandas/issues/31208

Labels:
name: Bug
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
LGTM
thanks @TomAugspurger
COMPAT: Argsort position matches NumPy
State: closed

Closes https://github.com/pandas-dev/pandas/issues/29884

Bit of a WIP, CI is probably going to fail here. One API question, do we want NaT to sort at the end, regardless of the NumPy version? Or should we match the behavior of the installed NumPy (NaT at the start for <1.18, at the end for >=1.18)?

Labels:
name: Datetime
description: Datetime data dtype
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
Perf looks OK I think

```python
import pandas as pd
import numpy as np

arr = np.arange(10000)
np.random.shuffle(arr)
idx = pd.DatetimeIndex(arr)

%timeit idx.sort_values()

idx = pd.PeriodIndex(ordinal=arr, freq='D')
%timeit idx.sort_values()
```

Master

```
403 µs ± 10.6 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
450 µs ± 9.41 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
```

PR

```
421 µs ± 3.39 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
468 µs ± 8.29 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
```

Not sure why they aren't identical yet. We're a bit outside the standard deviation on those runs.
> Not sure why they aren't identical yet. We're a bit outside the standard deviation on those runs.

i think we get one, maybe two extra function-call-overheads; does that account for 15-20 microseconds?
> im a bit confused by this. how is this prevented from varying based on the numpy version?

That was my [original question](https://github.com/pandas-dev/pandas/pull/31210#issue-365909033). Do we want to match the installed version of NumPy so that `np.sort(ndarray[m8[ns]])` matches `np.sort(DatetimeIndex), regardless of the NumPy versoin`? Or  do we want to adopt the new behavior of sorting last regardless of the NumPy version?

Both seem quite difficult, given that we sometimes do things like sort the datetimes, extract the i8 values, and then operate on them assuming they're sorted. I'm not confident that I can get this done for 1.0, if ever.
> I'm not confident that I can get this done for 1.0, if ever.

This does sound like the kind of problem that goes away on its own once our numpy min-version catches up.  I lean towards "dont sweat it"
I'm going to close this to get it out of the way.
@TomAugspurger what was missing / wrong / not yet working in the current PR? (or just still a lot of failing tests that need further investigation?)
Anything that relied on `.sort_values().asi8` to be sorted was broken / incorrect. This includes `.rolling`, `.resample`, etc.
Also affected: the ability to have DatetimeIndexOpsMixin.argsort dispatch to self._data.argsort
Ah, that indeed sounds more complicated :)
Backport PR #31017 on branch 1.0.x (REG: restore format_type attr)
State: closed

Backport PR #31017: REG: restore format_type attr

Labels:
name: Regression
description: Functionality that used to work in a prior pandas version
name: IO HDF5
description: read_hdf, HDFStore

Comments:
CI failure will be fixed by https://github.com/pandas-dev/pandas/pull/31209. OK to merge after that's merged, and then the merge commit should pass.
TST: More regression tests
State: closed

- [x] closes #30986
- [x] closes #19020
- [x] closes #19974
- [x] closes #18265
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`


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

Comments:
Thanks @mroeschke