BrandonFors/pandas-github-issues huggingface dataset:
ERR: better error message on unparseable datetimes
State: closed

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

Labels:
name: Datetime
description: Datetime data dtype
name: Error Reporting
description: Incorrect or improved errors from pandas
name: Timedelta
description: Timedelta data type

Comments:
Hello @baevpetr! 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-01-21 11:51:47 UTC
So, Linux py36_minimum_versions & macOS py36_macos builds do not pass the test:
their exceptions don't seem to contain the passing date.
@baevpetr can you continue pushing to #30711 instead of opening a new PR.

@simonjayhawkins, no, I can't. Sorry.
![image](https://user-images.githubusercontent.com/16845495/72663646-09fcd200-3a06-11ea-9be3-4b07d8053be6.png)

If you now unable to reinstate the original repository, then you should put a note in the original PR to make the reviewers of the original PR aware. To help maintain the history, should also update the OP to reference the previous PR.
And suddenly `pip-20` problems with installation in CI...
Was released several hours ago.
@baevpetr is this still active? Can you try to get CI green?
Closing as stale but ping if you'd like to pick back up
BUG: pd.concat fails when handling MultiIndex if repeated index exists
State: closed

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


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

Comments:
Can you check benchmarks for this? I would think that replacing the internal factorize with itertools.chain could drastically slow things down
thanks! @WillAyd changed

btw, just a question, somehow, yesterday when i was trying to check benchmark, it takes ages and I felt my laptop almost burned, is this normal for local benchmark checking?

I will check the benchmark tomorrow
Running the entire benchmark suite will take a while. You probably want to limit it to what you think is the affected code, so something like `asv continuous upstream/master HEAD -b join_merge.Concat` for this PR 

Hello @charlesdong1991! 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-04-13 09:00:15 UTC
@charlesdong1991 still active? Can you fix merge conflicts and try to get CI green?
yeah, still active, but i will be off for vacation for several days, and this needs some work so will update in a week. @WillAyd 👍 
@charlesdong1991 what's the status here?
emm, let me close for now, haven't past tests locally. will reopen once finding a way.
BUG: Index.get_value being called incorrectly
State: closed

AFAICT there aren't user-facing consequences, just built-up kludges in `Series.__getitem__` and `Index.get_value` that we'll be able to get rid of once we fix these inconsistencies.

cc @jreback

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

Comments:
yep, these have been bulit up over time +1 for attaching :->
POC: docstring inheritance
State: closed

Proof of concept for automatically inheriting docstrings, so we dont have to use `@Appender(parent_class.method.__doc__)` quite so much.

This doesnt handle `@Substitution`.  If we can make that work for the common case where we're just putting in the class name, we could get a _lot_ less verbose.

xref #31095, #31060.

Labels:
name: Docs

Comments:
Looks nice though I am actually -1 on inheriting docstrings compared to simplifying the existing Appender / Substitution decorators. I suppose my main argument is that not everything really can be inherited (ex: we sometimes inherit but change signature or semantics) so would end up having to revert back to Appender / Substitution anyway in some cases
Closing to clear the queue
CLN: prune unreachable code
State: closed


Labels:
name: Clean

Comments:
I'm all for simplifying code, but I'm not really seeing how any of the code deleted was "unreachable".  Is there a previous PR or issue that impelled this?
> but I'm not really seeing how any of the code deleted was "unreachable"

For private/internal code, we can just look at all the places where it is called and determine that e.g. ShallowMixin._shallow_copy always has `obj` passed.
thanks @jbrockmendel
TST: Fix some bare pytest raises
State: closed

- [x] ref #30999
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Follow up for #30998

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

Comments:
@MomIsBestFriend might be misreading but I don't really understand this PR. Is this de-parametrizing an existing test?
> @MomIsBestFriend might be misreading but I don't really understand this PR. Is this de-parametrizing an existing test?

Correct, because I had troubles in #30998 to put a correct "match" value (trying to remove bare pytest raises statement).

I was having something that looked kinda like this:
```
msg = (
              r"cannot perform __pow\_\_ with this index type: DatetimeArray|"
              r"cannot perform __mod\_\_ with this index type: DatetimeArray|"
              r"cannot perform __truediv\_\_ with this index type: DatetimeArray|"
              r"cannot perform __mul\_\_ with this index type: DatetimeArray|"
              r"cannot perform __pow\_\_ with this index type: TimedeltaArray|"
               "ufunc 'multiply' cannot use operands with types dtype"
              r"\('<m8\[ns\]'\) and dtype\('<m8\[ns\]'\)|"
                "cannot add DatetimeArray and Timestamp"
         )
```
(That value did not work)

I think, IMO the "de-parameterization" functions is more readable and maintainable (And I was able to fix the bare pytests raises statement).
I think should keep the original parametrization. The error messages are of secondary importance so don't need to be exact. You can just pick a simpler pattern like `cannot (perform|add)`
> I think should keep the original parametrization. The error messages are of secondary importance so don't need to be exact. You can just pick a simpler pattern like `cannot (perform|add)`

I know that ```msg``` (https://github.com/pandas-dev/pandas/pull/31105#issuecomment-575725159) can be prettier using some regex "magic".
The problem is that the parameterized test is also raising an external error message, which can be a bit trickier to "ignore" that specific message, thing that will probably that there will be the need for some nested ```if``` statements.

---

I will commit the parameterized test, with the nested ```if``` statement(s).
And just pick the one you like more :)

Also, side question, does the parameterized test is more performant? (trying to see why the de-parameterized test is less likeable)

  
> Also, side question, does the parameterized test is more performant?

I don't think it matters but even if it did should strongly prefer parametrized tests to breaking out individually; the latter is tougher to extend and maintain over time and leads to more lapses in coverage
@WillAyd You were 100% right!
 it's just one ```if``` statement, and it does looks much better than the de-parameterized tests
(unrelated) 
If someone mention you in a regular comment of a PR/issue that was merge/closed, would you still get notified?
Thanks @MomIsBestFriend and answer to your question depends on per-user settings
CLN: Index._values docstring + Block.internal/external_values
State: closed

@jbrockmendel this is the "clean" part of https://github.com/pandas-dev/pandas/pull/31037, so if you merge this, I will update the other PR so that only the required changes are in the PR for 1.0.0.

Labels:
name: Clean

Comments:
LGTM
Is this all internal or would anything leak into `.values`?
This is mainly doc updates + removal of an unused internal keyword + duplicated definition. Should be no public consequence.
Great thanks @jorisvandenbossche
BUG: groupby transform fillna produces wrong result
State: closed

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


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

Comments:
Switched to use ``transformation_func`` defined in conftest.
Have noticed a few issues there.

1. Why is `corrwith` a transformation function? Shouldn't it be a reduction?
1. groupby transformation with `pad`, `backfill` and `cumcount` raises ``AttributeError: 'Series' object has no attribute 'xxx'``. Have ignored these two cases in the test for now. This issue is already mentioned in #27472.
Hello @fujiaxiang! 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-01-24 05:12:24 UTC
Thanks @fujiaxiang keep the PRs coming!
Updated years in LICENSE
State: closed

Really not sure about this change.

(Feel free to close this PR at any time, if this file should not be changed).

Labels:
name: Docs

Comments:
Not sure about this either. Maybe @TomAugspurger knows
Not sure either.
Maybe @wesm would know then
I would recommend splitting out multiple copyright lines.

I was being paid by AQR from 2008 until mid/late 2011 (first as a FTE and then as a contractor), so I think you can state AQR as a copyright holder from 2008 - 2011. 

Beyond 2011 I think you can simply say "Copyright 2011 - 2020 pandas contributors". The project uses a shared copyright model where copyright is owned by the individuals contributing code to the project. So that would be my recommendation

```
Copyright 2008 - 2011 AQR Capital Management, LLC
Copyright 2011 - 2020 pandas open source contributors
```

or similar
thanks @MomIsBestFriend
Backport PR #31091 on branch 1.0.x (CI/TST: fix failing tests in py37_np_dev)
State: closed

(Manual) Backport PR #31091 on branch 1.0.x (CI/TST: fix failing tests in py37_np_dev)

cc @WillAyd 

Labels:
name: CI
description: Continuous Integration

Comments:
Thanks!
CLN: Regular expression clean
State: closed

- [x] ref https://github.com/pandas-dev/pandas/pull/31091#discussion_r367732954
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry



Labels:
name: Clean

Comments:
Thanks @MomIsBestFriend
ENH: partial string indexing on non-monotonic PeriodIndex
State: closed

Looks like the DatetimeIndex analogue of this was done in #2437 (https://github.com/pandas-dev/pandas/commit/3a173f19b308d2fb7d8e96dfc57f2e5ecd046138)

This does _not_ yet support two-sides slicing e.g. `ser["2015":"2016"]`, but im hoping to get that handled before long.

Partial overlap with #31058, nothing should actually conflict.

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

Comments:
Is this a bug fix or a new feature? Can you add a whatsnew?
> Is this a bug fix or a new feature? Can you add a whatsnew?

I think it counts as a feature, but there are a lot of inconsistencies in the surrounding code, so I wouldn't argue the point.  Just added whatsnew.
can you rebase
one more rebase
rebased+green
Thanks @jbrockmendel
Pull Request Tips
State: closed

I think these issues pop up on a good deal of PRs but aren't explicitly stated anywhere, so adding a few bullet points to the contributing guide might help

Labels:
name: Docs

Comments:
LGTM
yep good idea. maybe also something about raising an issue for discussion before submitting a pull request if there is not already an issue on the subject.
Follow-up: I would link to this document in the PR template.
thanks @WillAyd 

if you can followup with @gfyoung suggestion would be great.
CLN: update _simple_new usages
State: closed

In preparation for making the _simple_new constructors stricter, xref #31084, #31055.

Labels:
name: Clean
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.
name: Constructors
description: Series/DataFrame/Index/pd.array Constructors

Comments:
pre-cursor to #31055 ?
> pre-cursor to #31055 ?

No, #31055 should be independent of this.  But there are a few more PRs similar to 31055 coming up, and this is a precursor to some of those (and/or makes the diffs more focused)
DOC: automatic 'end' year of copyright
State: closed

#31022 Updated it after nearly 6 years, I think this will update with each new merge, so that should not be a problem from now on, (I think)
cc (@datapythonista)


Labels:
name: Docs

Comments:
Thanks @MomIsBestFriend 
> Makes sense, looks good to me.
> 
> In general I think it's better to `import datetime`, since when reading code `datetime` is ambiguous, but `datetime.datetime` is not. But not a big deal, we have imports like this everywhere in the code, just saying.

Do you mind if I open a pull request addressing these?
Better open an issue first, so there is some discussion and you see if there is agreement in it.
CLN: __all__ for core/index.py
State: closed

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


Labels:
name: Clean

Comments:
this file is not used __anywhere__ in the code base (AT ALL).
It was deprecated in #30193 (About a month ago).

Is it OK to removed it completely?

I don't know if a month is a lot of time or not when it comes to API changes, if it's not a lot of time, then what is a good time span to give users a warn about API change before actually changing it?
The CI is red, I guess the objects you removed were being imported somewhere.
Looks like it failed before it began the tests, closing and reopening
Sorry, assumed there were genuine test fails. The failure is a correct error now, you need to either remove `_new_Index` if it's not used, or add it to the `__all__` (with a comment if possible).
This is a deprecated module that is being changed, that should not be used in pandas. So what is the rationale for this change?
> This is a deprecated module that is being changed, that should not be used in pandas. So what is the rationale for this change?

@jorisvandenbossche I just saw the ```# noqa:F401``` comments, and decided to cleaned it up, regardless if the module is deprecated or not.
OK, I wouldn't put effort in a deprecated / unused file, but too late I guess :-)

Is there a general issue about such changes?
> but too late I guess :-)

That's exactly what I said, when I saw the deprecation message... only after I have made the changes :)

> Is there a general issue about such changes?

There isn't a "master issue" for it, it's something I started to do here and there since https://github.com/pandas-dev/pandas/pull/30828#discussion_r364500688 was pointed out.

Yea I think since deprecated maybe not even worth touching this module so just closing this one out. But thanks as always @MomIsBestFriend 
Understandable :)
TST: Remove bare pytest.raises 
State: closed

References #30999 

Adds match argument to pytest.raises found in the following files. https://github.com/pandas-dev/pandas/issues/30999#issuecomment-574174389

**ToDo:**
- [x] pandas/tests/arithmetic/test_numeric
- [X] pandas/tests/arithmetic/test_object
- [x] pandas/tests/arithmetic/test_timedelta64
- [x] pandas/tests/arrays/interval/test_interval
- [ ] add whatsnew entry

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

Comments:
Hello @gdex1! 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-01-21 15:20:05 UTC
Could someone give me some direction on resolving the failed integration tests?  I do not understand how changing a string to an f-string in my latest commit would cause the error:
`Git checkout failed with exit code: 128`.
I think GitHub CI was temporarily down; just restarted let's see
thanks @gdex1 
@MomIsBestFriend .  I've finished adding match arguments for pytest.raises calls in these files.  Should I add an entry to the "whatsnew" file?  Or will there be one added later related to the original issue you opened?  Thanks.
@gdex1 First, thank you for contributing:)

And the "what's new" is a file that's displayed to users who wants to learn what's changed.

the tests are for us to test the code, the external users do not use these tests and they also don't see these tests, so no you should not create a what's new file.

And there will be no new entry for the related issue :)

---

Side note:
Don't be afraid to do mistakes (that's how you learn).
I'm sure, if you open a PR (related to this issue) with a what's new entry, you'll receive feedback (and better one).

---

:)
CLN: remove unused legacy pickle compat code
State: closed

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


Labels:
name: Clean

Comments:
thanks @jbrockmendel 
hit wrong milestone
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 14ab82eba54a7369698020aae6487ea96f7ac20e
```

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

```
$ git commit -am 'Backport PR #31078: CLN: remove unused legacy pickle compat code'
```

4. Push to a named branch :

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

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

> "Backport PR #31078 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).
STY: Some stuff that triggers my OCD ;)
State: closed

- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

 Some stuff that got the attention of my eye

Labels:
name: Clean

Comments:
Looks like it got disconnected from the internet.

Closing && reopening just to be safe. 
Thanks @MomIsBestFriend
ENH: accept a dictionary in plot colors
State: closed

This is a resurrection of #28659. Have kept the original commit history, just rebased and added docstrings.

- [x] closes #8193
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Visualization
description: plotting

Comments:
@jreback have done some docstring-sharing, here are some screenshots:

![image](https://user-images.githubusercontent.com/33491632/72686299-07c07380-3aeb-11ea-9818-aa9144dbd7bc.png)
![image](https://user-images.githubusercontent.com/33491632/72686306-29b9f600-3aeb-11ea-8212-7caaac0bc27b.png)
![image](https://user-images.githubusercontent.com/33491632/72686322-4c4c0f00-3aeb-11ea-9600-e1caf0b0f70a.png)
![image](https://user-images.githubusercontent.com/33491632/72686335-7ac9ea00-3aeb-11ea-953b-1bb98aa28b42.png)
![image](https://user-images.githubusercontent.com/33491632/72686345-959c5e80-3aeb-11ea-9661-1e34d8edc6ea.png)
![image](https://user-images.githubusercontent.com/33491632/72686365-cda3a180-3aeb-11ea-937a-c08ebbed4f4e.png)

great lgtm. if @datapythonista or @simonjayhawkins have any comments on the doc-strings otherwise merge away.
thanks @MarcoGorelli 
Happy to help, thanks @Leostayner for having started this
PERF: add shortcut to Timedelta constructor
State: closed

WIP until #30676 gets merged.
- [ ] second half of #30543
- [X] tests added 1 / passed 1
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry

This implements a shortcut in the Timedelta constructor to cut down on processing if Timedelta is passed. We still need to check if any other args are passed. Then, if a Timedelta with no other kwargs was passed, we just return that same Timedelta.
A test is added to check that the Timedelta is still the same object.

Labels:
name: Performance
description: Memory or execution speed performance
name: Timedelta
description: Timedelta data type

Comments:
Same as in the Timestamp shortcut, we could hoist the shortcut to the very top of `__new__`, but I don't think that's necessary. Putting it before error checks looks messy to me, and the performance improvement in any real code would be miniscule (as casting the object into its own class is unlikely to be a significant part of a program). When running `timeit`, the option with the shortcut at the very top provided about a 20% performance boost.
What kind of performance gain does this show?
@WillAyd About 20% on 10 000 000 iterations.
thanks @AlexKirko
DOC: add plotting backends in visualization.rst
State: closed

- [x] closes #30984 


Labels:
name: Docs

Comments:
@jorisvandenbossche 
> There is just one thing I don't see clear. I think we may be adding content for two different audiences here. I think in the visualization user guide it makes sense to explain that other plotting backends exist, and how to use them. But I think the last part is not for pandas users, but for developers of pandas extensions. I think this should be in the Development documentation. 

Yes, what this PR is adding is meant for users, the development part already exists: https://dev.pandas.io/docs/development/extending.html#plotting-backends. See also my first review comment to remove this part here in the PR (and replace it with a link, what you said :-))
Saw the other comments but missed that one. Agree with all points. :) 
Can you review this PR, please?
thanks, @jorisvandenbossche for those corrections. The linting is clear now.
does it require any change now?
Thanks!
TYP/CLN: Replaced "Optional[Hashable]" with "Label" from pandas._typing
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: Typing
description: type annotations, mypy/pyright type checking

Comments:
Thanks @MomIsBestFriend
CLN: Removed unused varibles from for loops
State: closed

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


Labels:
name: Clean

Comments:
Can we remove the cdef declarations for `i`?
> Can we remove the cdef declarations for `i`?

@jbrockmendel No, There's is uses for it in some of the for-loops.
Thanks @MomIsBestFriend