BrandonFors/pandas-github-issues huggingface dataset:
implemented additionally functionality of formatters_col in to_latex
State: closed

- [ x] closes #26278
- [ x] tests added / passed
- [ x ] passes `black pandas`
- [ x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ x] whatsnew entry

Was having some issues rebasing on https://github.com/pandas-dev/pandas/pull/32666 so making a new PR with same changes. Closing other PR as well. 

Labels:
name: Enhancement
name: IO LaTeX
description: to_latex
name: Stale

Comments:
cc @WillAyd @mroeschke 
cc @ivanovmg if you'd have a look
> I agree with Jeff on moving the formatters logic into DataFrameFormatter.
> 
> There is a method ``_initialize_formatters``, which can be updated.
> 
> However, it will alter formatters not only for latex output, but for others as well (to_string, to_html, to_csv).
> 
> Is that ok?

i think this is ok but we should have some additional tests in place 
> > I agree with Jeff on moving the formatters logic into DataFrameFormatter.
> > There is a method `_initialize_formatters`, which can be updated.
> > However, it will alter formatters not only for latex output, but for others as well (to_string, to_html, to_csv).
> > Is that ok?
> 
> i think this is ok but we should have some additional tests in place

Which tests did you have in mind? I think the FormattersType change looks self contained with the DataFrameFormatter class
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 as stale. if you want to continue, pls ping and can re-open.
ENH: Improve error reporting for wrong merge cols
State: closed

- [x] closes #16228
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Fixed error reporting when double conditions are given. Additionally I found, that something like 
```
left = DataFrame({"a": [1, 2], "b": [3, 4]})
right = DataFrame({"a": [1, 1], "c": [5, 6]})
pd.merge(left, right, right_on="a")
```

raised an uncontrolled error:

```
Traceback (most recent call last):
  File "/home/developer/.config/JetBrains/PyCharm2020.2/scratches/scratch_4.py", line 25, in <module>
    print(pd.merge(left, right, right_on="a"))
  File "/home/developer/PycharmProjects/pandas/pandas/core/reshape/merge.py", line 72, in merge
    op = _MergeOperation(
  File "/home/developer/PycharmProjects/pandas/pandas/core/reshape/merge.py", line 643, in __init__
    self._validate_specification()
  File "/home/developer/PycharmProjects/pandas/pandas/core/reshape/merge.py", line 1247, in _validate_specification
    if len(self.right_on) != len(self.left_on):
TypeError: object of type 'NoneType' has no len()
```



Labels:
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Error Reporting
description: Incorrect or improved errors from pandas

Comments:
Unfortunately I don't have an idea how to simplify this significantly right now, but will try to look into this into the future.
thanks @phofl very nice, this has been an open issue for a while!

yeah happy to take refactoring PRs (but not sure this is that easy), but i am sure there are parts that can be cleaned.
ENH: Add dtype argument to read_sql_query (GH10285)
State: closed

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

Labels:
name: Enhancement
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: IO SQL
description: to_sql, read_sql, read_sql_query

Comments:
@jorisvandenbossche tagging you since you were involved in the original issue. Could you have a look at this imp and lmk whether this is what you had in mind?
@jorisvandenbossche could you (or maybe someone else) have a look at this PR? Thanks
@jreback could you (or maybe someone else) have a look at this PR? Thanks
Updated PR. Only Linux py38_np_dev fails, but due to other reasons.
Merged master, everything succeeds.

I also saw a similar enhancement request for dtypes for `read_sql`. Once we agree on this imp we can easily extend it to that function as well
@jreback could you have another look? Will fix the merge conflict in the docs later today
In #13049 it is proposed to add the `dtype` argument to `read_sql`. This can be done in similar manner as in this PR.
lgtm ping on greenish
as a followup :-> can type any `dtype=` args in the codebase
@jreback CI is greenish,Travis failed due to other reasons

Will work on dtype follow up tomorrow.
thanks @avinashpancham 

small comment for followon
Added example to the aggregate function
State: closed

Added example of k-hot encoding to the aggregate function

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:

Comments:
Hello @XtremeGood! 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:

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

> [Line 7496:5](https://github.com/pandas-dev/pandas/blob/1e08bee60bbefc18f37c1ced2ab595ed4dd2d832/pandas/core/frame.py#L7496): [E101](https://duckduckgo.com/?q=pep8%20E101) indentation contains mixed spaces and tabs
> [Line 7496:5](https://github.com/pandas-dev/pandas/blob/1e08bee60bbefc18f37c1ced2ab595ed4dd2d832/pandas/core/frame.py#L7496): [W191](https://duckduckgo.com/?q=pep8%20W191) indentation contains tabs
> [Line 7504:89](https://github.com/pandas-dev/pandas/blob/1e08bee60bbefc18f37c1ced2ab595ed4dd2d832/pandas/core/frame.py#L7504): [E501](https://duckduckgo.com/?q=pep8%20E501) line too long (106 > 88 characters)
> [Line 7511:56](https://github.com/pandas-dev/pandas/blob/1e08bee60bbefc18f37c1ced2ab595ed4dd2d832/pandas/core/frame.py#L7511): [W291](https://duckduckgo.com/?q=pep8%20W291) trailing whitespace
> [Line 7512:89](https://github.com/pandas-dev/pandas/blob/1e08bee60bbefc18f37c1ced2ab595ed4dd2d832/pandas/core/frame.py#L7512): [E501](https://duckduckgo.com/?q=pep8%20E501) line too long (101 > 88 characters)
> [Line 7518:31](https://github.com/pandas-dev/pandas/blob/1e08bee60bbefc18f37c1ced2ab595ed4dd2d832/pandas/core/frame.py#L7518): [W291](https://duckduckgo.com/?q=pep8%20W291) trailing whitespace



##### Comment last updated at 2020-10-31 21:24:13 UTC
> 
> 
> As noted in the issue I'm -1 on putting this in

No problems. I will find a real `Good First Coding` issue.
TYP: PoC for honest ABC
State: closed

PoC implementation for using honest ABCs for pandas objects. Currently only implements ABCNDFrame. Compare:

```
def foo(obj: ABCNDFrame):
    reveal_type(obj.attrs)


def bar(obj: ABCDataFrame):
    reveal_type(obj.attrs)
```

In this PR, mypy will output `Revealed type is 'builtins.dict[Union[typing.Hashable, None], Any]'` for the former whereas the latter is `Revealed type is 'Any'`.

A script can be written (and I plan to, if this idea gets buy-in) to generate the ABC class off of the concrete class. Likewise, tests can be written to ensure that ABCNDFrame and NDFrame don't diverge.

docstrings can be removed from the ABC class; I just wanted to highlight that we could just have the docs in the ABC class (don't know if that would be desirable).

cc @simonjayhawkins, @jreback, @jbrockmendel 

Labels:

Comments:
This is a big change.  Why not just put in dtypes.generic

```
class ABCNDFrame:
    pass
```

and then "honestly" have NDFrame subclass that?
The idea here is to allow mypy to be aware of method signatures when doing e.g. `isinstance(obj, ABCNDFrame)`. I believe this is only possible by copying the signatures of the methods of `NDFrame` into `ABCNDFrame`.

The diff will go down dramatically if docstrings are removed.
I removed docstrings to make the diff easier to understand.
related issue #27353

> A script can be written (and I plan to, if this idea gets buy-in) to generate the ABC class off of the concrete class. Likewise, tests can be written to ensure that ABCNDFrame and NDFrame don't diverge.

stubgen can generate stub files automatically. 

I think we should maybe investigate creating stub files in the first instance, see https://github.com/pandas-dev/pandas/issues/27353#issuecomment-526391462

These may need to be manually curated in the first instance. But I think using stubgen could reduce the need for custom scripts and checks longer term.
@simonjayhawkins Thanks for the comments and links. Using cast on the ABCs did not occur to me, even if we decide not to go with stubs it seems clear to me that cast is a better solution than what is being done here.
TST: split up tests/plotting/test_frame.py into subdir & modules #34769
State: closed

- [ ] closes #34769
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

I moved the file to a new directory.
What is the best structure for this folder?

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

Comments:
you could start with groupby, subplots, colors 

just look for a grouping of tests that are common and move them. i would recommend merge master on this and can merge, then do followup PRs to extract.
@jreback OK thanks.

I think I figured out what to do with the tests.
But I didn't understand the merge conflict. The merge goes fine locally.
make sure to `git pull upstream/master`
Hello @Mikhaylov-yv! 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-02 17:32:53 UTC
Am I doing the right thing?
yes this looks fine. can you show the before / after test runs for this dir (make sure test counts don't change).

ping when ready. btw no need to do anymore in this PR, can followup with others. ping on green.
Oh no. I did something wrong.
excuse me

Probably better to create a new PR and delete this one?
#37535 pandas-CI: revert disable of windows window tests
State: closed

- [x] closes #37535 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry
removed the restrictions on windows test


Labels:

Comments:
@nagesh-chowdaiah the point is to actually fix this. We don't know what is wrong.
BUG: error raise when column contains percentage
State: closed

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


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

Comments:
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.
> lgtm but needs to be looked over by someone who's more familiar with the sql module
> 
> @erfannariman would you mind merging master again to make sure this up to date

Done
this was deliberately changed in this was deliberately changed in #34212 so not sure why you think this is a regression.
thanks @erfannariman 
@meeseeksdev backport 1.2.x
TYP: obj in aggregation.py
State: closed

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

Labels:
name: Apply
description: Apply, Aggregate, Transform, Map
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
hmm failing some ci/checks
lgtm merge on green
CLN/TYP: Alias for aggregation dictionary argument
State: closed

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


Labels:
name: Apply
description: Apply, Aggregate, Transform, Map
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
thanks @rhshadrach
CLN: Use ABCNDFrame instead of ABCDataFrame and ABCSeries
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:
Thanks @jbrockmendel, for the pivot case, `NDFrame` was okay to import as well. My thinking is that `isinstance(obj, NDFrame)` is preferred over `isinstance(obj, (Series, DataFrame)`.
BUG: isin incorrectly casting ints to datetimes
State: closed

- [x] closes #36621
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

De-duplication in follow-up.

Labels:
name: Bug
name: Datetime
description: Datetime data dtype
name: Algos
description: Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff

Comments:
> this going to need perf tests as this is very sensitive

will run the appropriate asvs
actually, i think this can be simplified to just use _from_sequence following #37179
gentle ping.  id like to see if we can share some code between this and `equals`
Found we don't have any asvs that hit this, so just added a few.  No change for cases with matching dtype, nice pickup for others:

```
       before           after         ratio
     [f823a859]       [9758b021]
     <ref-dtlike-indexers>       <bug-dti-isin>
-        691±30μs          143±3μs     0.21  series_methods.IsInDatetime64.time_isin_empty
-     1.23±0.08ms          141±8μs     0.11  series_methods.IsInDatetime64.time_isin_mismatched_dtype

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.

```
I guess as a short term measure this is ok as you fixed some bugs. the problem here is that you are bouncing between algos and the indexes, where everything should be in algos. this feels like a step backwards, if its tactical, then ok.
> the problem here is that you are bouncing between algos and the indexes, where everything should be in algos

its not unusual for core.algorithms functions to call EA methods.  There's no way the casting logic belongs in core.algorithms.
> > the problem here is that you are bouncing between algos and the indexes, where everything should be in algos
> 
> its not unusual for core.algorithms functions to call EA methods. There's no way the casting logic belongs in core.algorithms.

that's not what's happening here, these are calling Index methods. This seems quite unusual and a very odd depenency.
Why do you not simply move the logic in DTI.isin back to algorithms? what's the point of adding this depenedecy.?
> that's not what's happening here, these are calling Index methods

Good point, my mistake.  At some point this should be consolidated in the EA.  IIRC theres an issue about adding `isin` to the interface.

> Why do you not simply move the logic in DTI.isin back to algorithms? 

because the relevant logic is _casting_ logic that belongs in the DTA.  My current thought is that once we get the _from_sequence-strictness PR in we'll be able to de-duplicate some logic between isin+equals using that.


The new hashtables have me looking at algorithms._ensure_data, and I'm pretty confident that this will allow us to simplify it quite a bit
BUG: slice_canonize incorrectly raising
State: closed

- [ ] closes #xxxx
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Doesn't appear to affect anything in master, but stumbled on it when implementing the PR do always return views when indexing on columns.

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

Comments:
sure merge on green
TST: setting value at MultiIndex slice using .loc
State: closed

- [x] closes  #34870
- [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
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: MultiIndex

Comments:
thanks @arw2019
REGR: inplace Series op not actually operating inplace
State: closed

- [x] closes #36373
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

alternative to #37497, ensures that Series inplace ops are actually inplace whenever possible.  The whatsnew is copied verbatim from #37497, the new test is copied with an additional assertion about `series._values`

Labels:
name: Regression
description: Functionality that used to work in a prior pandas version
name: Copy / view semantics

Comments:
@MeeseeksDev backport to 1.1.x
Owee, I'm MrMeeseeks, Look at me.

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

1. Checkout backport branch and update it.

```
$ git checkout 1.1.x
$ git pull
```

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

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

```
$ git commit -am 'Backport PR #37508: REGR: inplace Series op not actually operating inplace'
```

4. Push to a named branch :

```
git push YOURFORK 1.1.x:auto-backport-of-pr-37508-on-1.1.x
```

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

> "Backport PR #37508 on branch 1.1.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).
REGR: revert behaviour of getitem with assigning with a Series
State: closed

Closes #37427

@jbrockmendel this is reverting part of the clean-up you did in https://github.com/pandas-dev/pandas/pull/33643. I think the behavioural change was unintentional in that PR. We might still want to do it for 1.2, but then with a deprecation first.

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Regression
description: Functionality that used to work in a prior pandas version

Comments:
@MeeseeksDev backport to 1.1.x
Owee, I'm MrMeeseeks, Look at me.

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

1. Checkout backport branch and update it.

```
$ git checkout 1.1.x
$ git pull
```

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

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

```
$ git commit -am 'Backport PR #37502: REGR: revert behaviour of getitem with assigning with a Series'
```

4. Push to a named branch :

```
git push YOURFORK 1.1.x:auto-backport-of-pr-37502-on-1.1.x
```

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

> "Backport PR #37502 on branch 1.1.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).
REGR: fix isin for large series with nan and mixed object dtype (causing regression in read_csv)
State: closed

Closes #37094


Labels:
name: Regression
description: Functionality that used to work in a prior pandas version
name: IO CSV
description: read_csv, to_csv
name: Algos
description: Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff

Comments:
@simonjayhawkins ok for you?
@meeseeksdev backport 1.1.x
REGR: fix inplace arithmetic operation on Series no longer updating parent DataFrame
State: closed

Closes #36373

See discussion at https://github.com/pandas-dev/pandas/pull/36498#issuecomment-708578774 for reverting the original fix (https://github.com/pandas-dev/pandas/pull/30501 for #30484)

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

Comments:
@jorisvandenbossche i have no objection if you're just looking for the quickest available fix, but another option is to update NDFrame._inplace_method:

```
def _inplace_method(self, other, op):
    result = op(self, other)

+    if self.ndim == 1 and result._indexed_same(self) and result.dtype == self.dtype:
+        self._values[:] = result._values
+        return self

    self._reset_cacher()
    [...]
```

i havent run the full test suite on this yet, but it passes the new test this PR adds and one of the two that this PR xfails.
> i have no objection if you're just looking for the quickest available fix

That's indeed the case, as I want to have this fixed for 1.1.4, and I am almost off to bed ;-) 
But certainly feel free to push that change here (or in another PR), if it passes the test we can merge that instead.
 
yeah let's just revert this as is to avoid blocking. @jbrockmendel feel free to push your change in a new PR
> @jbrockmendel feel free to push your change in a new PR

or could add the change to _maybe_cache_changed from #36498. The blockers on that PR weren't related to that change.
Closing in favor of https://github.com/pandas-dev/pandas/pull/37508, which fixes it without reverting the fix in `drop` (thanks @jbrockmendel !)
TST/REF: loc/iloc/at/iat tests go in tests.indexing
State: closed

not tests.(series|frame).indexing

`(Series|DataFrame).__(getitem|setitem)__` tests go in the tests.(series|frame).indexing



Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves

Comments:
sure I would add a README.me i think to tests/frame & tests/series to indicate the test structure a bit more (e.g. the methods/ dir) and the fact that all indexing tests go in test/indexing (alt we can have a top-level readme do this).
yah ive got a half-written file to that effect