BrandonFors/pandas-github-issues huggingface dataset:
DEPR: Deprecate tshift and integrate it to shift
State: closed

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


Labels:
name: Datetime
description: Datetime data dtype
name: Deprecate
description: Functionality to remove in pandas

Comments:
thanks. would not be averse to actually re-writing tshift in terms of shift; I *think* this would be straightforward. Also could merge this and then followon to do that.
> would not be averse to actually re-writing tshift in terms of shift; I think this would be straightforward

Agree it's rather straightforward, but just to clarify a few things: 
1. we are still going to deprecate `tshift` after the re-write right? In other words, you don't mean to re-write and **keep** `tshift` right?
1. do you think it's better to move the current `_tshift` inside `shift`? i.e. we won't have a separate method `_tshift`, but unpack what's in it and put inside `shift`
> > would not be averse to actually re-writing tshift in terms of shift; I think this would be straightforward
> 
> Agree it's rather straightforward, but just to clarify a few things:
> 
> 1. we are still going to deprecate `tshift` after the re-write right? In other words, you don't mean to re-write and **keep** `tshift` right?

correct, still deprecate

> 2. do you think it's better to move the current `_tshift` inside `shift`? i.e. we won't have a separate method `_tshift`, but unpack what's in it and put inside `shift`

yes if possible (ideally simplified as much as possible).


Have unpacked previous `_tshift` and moved into `shift`, also simplified the code a little bit. The deprecating `tshift` now calls `shift`. @jreback can you review?
@fujiaxiang can you rebase and some small comments, ping on green.
@jreback have split up the tests into PI and DTI and also separated the failure tests
I tried to parametrize the test cases into PI and DTI but I realize they only have a very small proportion of code in common, so I decided to separate them into different tests
> @jreback have split up the tests into PI and DTI and also separated the failure tests
> I tried to parametrize the test cases into PI and DTI but I realize they only have a very small proportion of code in common, so I decided to separate them into different tests

kk sounds good. ping on green (note that there maybe be a failure on master that is unrelated)
thanks @fujiaxiang very nice
REF: avoid use of to_perioddelta
State: closed

tslibs is nominally self-contained, but several offsets methods still rely on DatetimeArray/PeriodArray behavior.  This is the first of several steps whittling that away.

Labels:
name: Timedelta
description: Timedelta data type
name: Period
description: Period data type
name: Clean

Comments:
sure can we completely remove to_perioddelta?
> sure can we completely remove to_perioddelta?

hopefully, but the remaining usages aren't as striaghtforward. xref #34555 moving in that direction.
REF: add to_offset to tslibs namespace
State: closed

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


Labels:
name: Frequency
description: DateOffsets
name: API - Consistency
description: Internal Consistency of API/Behavior

Comments:
great, this has been a long time coming.
green
CI/TST #34131 fixed test_floordiv_axis0_numexpr_path
State: closed

xref #34131 

- [ x ] passes `black pandas`
- [ x ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Test `/pandas/tests/frame/test_floordiv_axis0_numexpr_path` is slow. It builds a df with 10^6 elements to tests functions `floordiv` and `pow`. The number of elements can be reduced with keeping the same min/max values inside the dataframe.

Before/after timeit comparison:
```
26.6 s ± 1.04 s per loop (mean ± std. dev. of 7 runs, 1 loop each)
37.6 ms ± 3.45 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)
```

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

Comments:
I did it : green light :-D I had hard time with it !!!

Summary:
The tests are skipped if numexpr is not installed. Otherwise, the entry dataframe is built to construct the minimum of element to reduce the duration of the test.
this is ok, ping on green.
thanks @OlivierLuG
DEPR: tz kwarg in Period.to_timestamp
State: closed

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


Labels:
name: Timezones
description: Timezone data dtype
name: Period
description: Period data type
name: Deprecate
description: Functionality to remove in pandas

Comments:
pls add this to the deprecation removal issue
> Is there a motivation?

As mentioned in the OP, its inconsistent with the PeriodArray/PeriodIndex methods.
Whoops, sorry, missed that sentence.

Now, this keyword doesn't seem really a burden to me, so I still don't find that a fully convincing reason (could it be added to the array method?)

Also, it seems we are using this feature internally in pandas, according to the test failure.
> Also, it seems we are using this feature internally in pandas, according to the test failure.

It's in `timeseries.py`. It can of course easily be fixed there to use `tz_localize`
> Now, this keyword doesn't seem really a burden to me, so I still don't find that a fully convincing reason (could it be added to the array method?)

Either way works.  My default is to prefer a smaller API surface, but not a strong preferenc.

Note also that Period.to_timestamp incorrectly ignores the tz arg in the how == 'end' case, so i see it as deprecating a broken feature.
agree with @jbrockmendel here; simpler is better, we *always* convert to naive, which is inline the model of Periods which are naive by definition.
OK, if it's only partly implemented, that's certainly another reason to deprecate it
TST #22663: Operation between DataFrame with non-numeric types and incomplete series
State: closed

- [ x ] closes #22663
- [ 1 ] 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
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations

Comments:
thanks @OlivierLuG keep em coming!
CI: Linux py37_np_dev failing on 1.0.x
State: closed

xref https://github.com/pandas-dev/pandas/pull/34503#issuecomment-636651219

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

Comments:
LGTM
Travis is actually green, icon hasnt caught up.  merging
thanks @simonjayhawkins
BUG: asymmetric error bars for series (GH9536)
State: closed

closes #9536

This fix enables asymmetric errors bars for pd.Series plots. In the current implementation the bars are symmetric, even if two sets of errors are provided.

Took inspirations from #12046

Labels:
name: Visualization
description: plotting

Comments:
Hello @Steffen911! 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-07-17 13:40:41 UTC
@MarcoGorelli @datapythonista Could one of you review the PR? Or maybe suggest another reviewer? 
lgtm can u merge master and ping on green
lgtm ping on green
@jreback All tests are green. Travis reports itself as "Passed" when I go to the "Details", but apparently it didn't update the PR check after it has finished.
Thanks @Steffen911!
Update : In DataFrame Class, Columns should not contain duplicate val…
State: closed

Also updated the Docstring.(#12991)

Labels:

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

* In the file [`pandas/core/frame.py`](https://github.com/pandas-dev/pandas/blob/ba0c4d7e2101a09bb97d7be08c698119c5d08c27/pandas/core/frame.py):

> [Line 360:70](https://github.com/pandas-dev/pandas/blob/ba0c4d7e2101a09bb97d7be08c698119c5d08c27/pandas/core/frame.py#L360): [W291](https://duckduckgo.com/?q=pep8%20W291) trailing whitespace
> [Line 447:1](https://github.com/pandas-dev/pandas/blob/ba0c4d7e2101a09bb97d7be08c698119c5d08c27/pandas/core/frame.py#L447): [W293](https://duckduckgo.com/?q=pep8%20W293) blank line contains whitespace
> [Line 448:9](https://github.com/pandas-dev/pandas/blob/ba0c4d7e2101a09bb97d7be08c698119c5d08c27/pandas/core/frame.py#L448): [E265](https://duckduckgo.com/?q=pep8%20E265) block comment should start with '# '
> [Line 449:20](https://github.com/pandas-dev/pandas/blob/ba0c4d7e2101a09bb97d7be08c698119c5d08c27/pandas/core/frame.py#L449): [E711](https://duckduckgo.com/?q=pep8%20E711) comparison to None should be 'if cond is not None:'


I don't think this is addressing the original issue. DataFrames can have columns with duplicate labels, the problem is when trying to modify only one.
Wouldn't it be better if pandas didn't allow us to have duplicate column names ? Having columns with same name would rather create confusion for us.
Plus, this may avoid the issue #12991
> Wouldn't it be better if pandas didn't allow us to have duplicate column names ? Having columns with same name would rather create confusion for us.
> Plus, this may avoid the issue #12991

Probably yes, there's a WIP PR about not allowing duplicate labels https://github.com/pandas-dev/pandas/pull/28394
Thanks @MohakGangwani

AFAICT, #28394 would already address the issue of optionally disallowing duplicate column labels. Suddenly raising an exception without any prior warning wouldn't be good form - furthermore, you would need to add tests for this new behaviour.

Closing for now then as this looks like it's inactive and probably not the solution pandas would want to go with
DOC: Docstring updated for DataFrame.equals
State: closed

- [ ] closes #34498 


Labels:
name: Docs

Comments:
Hello @freakypandit! 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-08-07 18:32:08 UTC
@freakypandit do you have time to finish this up, else I can pick it up.
@freakypandit can you fix the lint failure.
@simonjayhawkins I will try to do this. 
I fixed the trailing whitespace (that was the linting failure)
Thanks @freakypandit !
REF: use standard pattern in normalize_i8_timestamps
State: closed

From here the plan is to move to passing `int64_t*` which will allow us to share code between the scalar/vector versions of this function

Labels:
name: Datetime
description: Datetime data dtype
name: Clean

Comments:
This one seems to have caused a significant performance regression: https://pandas.pydata.org/speed/pandas/#timeseries.DatetimeAccessor.time_dt_accessor_normalize?p-t='UTC'&commits=4fcc6a66

Is that expected / accepted?
yah, i was surprised to see that the cython version is non-trivially slower than the numpy version.  i have one long-shot guess at a way to speed up the cython version, will try that out today.
Backport PR #34481 on branch 1.0.x (DOC: start 1.0.5)
State: closed

Backport PR #34481: DOC: start 1.0.5

Labels:

Comments:
cc @simonjayhawkins there are some failures on numpy dev build, like:

```
________ ERROR collecting pandas/tests/frame/indexing/test_indexing.py _________
pandas/tests/frame/indexing/test_indexing.py:31: in <module>
    class TestDataFrameIndexing:
pandas/tests/frame/indexing/test_indexing.py:80: in TestDataFrameIndexing
    DataFrame(columns=list("AB")),
pandas/core/frame.py:435: in __init__
    mgr = init_dict(data, index, columns, dtype=dtype)
pandas/core/internals/construction.py:239: in init_dict
    val = construct_1d_arraylike_from_scalar(np.nan, len(index), nan_dtype)
pandas/core/dtypes/cast.py:1440: in construct_1d_arraylike_from_scalar
    dtype = dtype.dtype
E   AttributeError: type object 'object' has no attribute 'dtype'
```

Did you already see that before?

But merging this, since it is unrelated to the whatsnew
> Did you already see that before?

no, but I _think_ should soon have a fix. This is not present on master, mainly down to the changes in #33725.

rather than try and backport all the changes in #33725, will raise a PR shortly with just the changes to fix CI on 1.0.x branch directly against the 1.0.x branch.
CLN: GH29547 format with f-strings
State: closed

replace .format() for f-strings in the following:

1. pandas/tests/series/indexing/test_numeric.py
2. pandas/tests/series/indexing/test_take.py
3. pandas/tests/series/indexing/test_where.py



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

Comments:
Hello @DanBasson! 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-06-20 16:23:55 UTC
lgtm. ping on green (the 3.9 biuld is failing but x that)
i don't understand what you are saying (i'm new...).
what is "ping on green"?
> i don't understand what you are saying (i'm new...).
> what is "ping on green"?

it means when the CI are all green, comment. note that the CI is currently having various issues.
@jreback 

the CI are all green.
i got some errors which i couldn't understand
just to make sure (this error should be patched), can you merge upstream master and then ping on green.
Turns out that was the problem.
thank you very much @jreback 
thanks @DanBasson
REF/PERF: PeriodDtype decouple from DateOffset
State: closed

ATM we define PeriodDtype in terms of DateOffsets, but this is a misnomer.  In fact, virtually every Period/PeriodArray method has to start off by taking its `.freq` and finding the corresponding integer code.  This makes the integer code itself into a dtype.  We lose a little bit of ground on the constructor, then make it back up in subsequent calls.

```
In [2]: per = pd.Period("2016Q1")                                                                                                                                                                   

In [3]: %timeit per.year
556 ns ± 13.3 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)    # <-- master
94.5 ns ± 1.36 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)  # <-- PR

In [4]: %timeit pd.Period("2016Q1")                   
21.2 µs ± 176 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)  # <-- master
25.8 µs ± 457 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)  # <-- PR
```

The constructor perf I think we can improve by eventually cutting the DateOffset out of the process altogether.

We'll also be able to de-duplicate a _bunch_ of other stuff: FreqGroup can be defined in terms of PeriodDypeCode, Resolution can be defined in terms of FreqGroup (xref #34462), we can avoid redundant definitions of the dtype codes in period.pyx, and a lot of the rest of libfrequencies becomes unnecessary.

In a follow-up I plan to mix the cython-space PeriodDtype into the core.dtypes PeriodDtype and we can get the same perf improvements in the PeriodArray methods.

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

Comments:
> The other methods of the PeriodDtype class don't seem used and/or tested?

yah not yet.  i can remove them until they are actually used i guess
> It needs another name IMO, since we already have another PeriodDtype class that is something else

Any suggestions?  (I plan to mix it into the core.dtypes PeriodDtype before long)
updated with a placeholder name PeriodPseudoDtype
Alternative could also be `PeriodCode` 
(but if you are still going to change it, might also not matter too much)
updated with docstring.  im increasingly confident this will (eventually, soon-ish) let us de-duplicate Resolution, FreqGroup, and some cdef constants we define in libperiod.  Maybe even NPY_DATETIMEUNIT depending on what numpy does
ping
BUG: Fix value setting in case of merging via index on one side and column on other side.
State: closed

- [x] xref #15692
- [x] xref #17257 
- [x] xref #28220
- [x] xref #28243
- [x] xref #33232
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Until now the `_maybe_add_join_keys` function changed the target column in the result, if the join was done over index on one side and the column on the other side. This resulted in taking values from the index and setting them for the target column, which explained the weird behavior in the issues referenced. Together with #34468 this will fix the issues. Both combined will transfer the merged index in these cases and the merged columns with the correct name. As of now this fix will only transfer the values, so the index will be wrong here.

We can not skip the method completly, because this would cause issues in case of the column is in both `DataFrames` we have to run through this steps to ensure that the target values are right. So I check if this is only contained in one side.

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

Comments:
> 
> 
> Wow, that's a lot of issues :)
> 
> Just making a small comment ahead of core devs' reviews

Yeah they all had the same underlying issue. The index think and this caused these errors. Hope we can find a solution to merge this :)
@jreback 

Do you think it is preferably to do #34468 and this PR together?
closing this. @phofl I am pretty sure you actually broke up the fixes into new PRs. if that is not the case, can you address a specific issue here.
DOC: Clarify behaviour of pd.merge
State: closed

- [x] Closes #34412 (also attempts to clarify #34273)
- [x] tests added / passed (passes `scripts/validate_docstrings.py`)
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry (Not needed)


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

Comments:
We should discuss first if this is the expected behavior. Imo that is pretty weird. The join is exactly the same for both cases, but the result is different. Does not seem consistent.
@jreback @phofl what should the expected behaviour be? Also, some guidance through this part of the code would help (I believe this is responsible for the behaviour):
https://github.com/pandas-dev/pandas/blob/433c9007781080658553fbef1a4d0c2813b404c0/pandas/core/reshape/merge.py#L876-L884
I think the index from left should be preserved in both cases no? So the second example is incorrect IMO
@Bharat123rox can you try to identify the code paths that cause this inconsistency? 
Closing for now as I don't think this is something that we should document; probably needs some more discussion on the original issue and a fix to the code instead
ENH: Add read_logfmt()
State: closed

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


Labels:

Comments:
Doesn't pass CI yet becasue conda-forge does not have `logfmt` package.

Waiting for it to be approved: https://github.com/conda-forge/staged-recipes/pull/11779
> can u open an issue for discussion about this first

Did not know it was required, [contributing guide](https://pandas.pydata.org/docs/development/contributing.html) asks to search for issue, but "Bug reports and enhancement requests" section does not discuss enhancement requests. Here is an issue: https://github.com/pandas-dev/pandas/issues/34501
DOC: Added documentation for building using pyenv
State: closed

Added documentation for building using pyenv. Discussed this PR with @WillAyd on gitter on May 20.

Also fixed up another line he mentioned (python setup.py build_ext --inplace -j 4
instead of:
python setup.py build_ext --inplace -j 0) for conda/venv/pyenv.

Have rebuilt the docs and ensured they build properly.


Labels:
name: Docs

Comments:
Fixed trailing whitespace and re-submitted.
Thanks @rorcores 

> and re-submitted.

For the future, if you add another commit and push it, the pull request will be updated, there's no need to open a new one
> Thanks @rorcores
> 
> > and re-submitted.
> 
> For the future, if you add another commit and push it, the pull request will be updated, there's no need to open a new one

Thanks @MarcoGorelli, really good to know.
@datapythonista care to look?
Thanks @rorcores
ENH: mul(Tick, float); simplify to_offset
State: closed

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

Labels:
name: Frequency
description: DateOffsets

Comments:
kk if u can rebase
rebased+green
DOC: updating the `indicator` wording in `merge` doc
State: closed

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


Labels:
name: Docs

Comments:
Hello @nnick14! 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-05-31 00:12:51 UTC
Thanks @nnick14