BrandonFors/pandas-github-issues huggingface dataset:
BUG/DEPR: loc.__setitem__ incorrectly accepting positional slices
State: closed

- [x] closes #26412
- [x] closes #16121
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

I plan to flesh out the tests before this goes in.

cc @toobaz 

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

Comments:
Since this has been such a long time behavior (also used in our own tests ..), I think we should maybe consider raising a warning first.
> I think we should maybe consider raising a warning first.

Yah, i'm leaning towards a deprecation cycle, too.  
can you rebase, and agree with @jorisvandenbossche this is worth deprecating (even though its an incorrect behavior)
updated per comments, but i think the "Do X instead" part isnt quite right.  "Use iloc instead" works for Series, but doesnt work as well for DataFrame
lgtm. can you add to the deprcations issue  & rebase; ping on green.
added to the deprecation tracker, rebased, and green (though the CI is broken for unrelated reasons)

However I would still like to get a better message for users suggesting an alternative to use for the DataFrame case.  cc @jorisvandenbossche for suggestions?
@WillAyd suggestions for a more helpful message for the 2D case?
> I'd say something like "use .loc with labels, or .iloc with positions instead." I don't think we can really say which will be easier for the user in the error message, so just don't take a position.

+1 on that. Trying to explain both options more in detail will get a bit complex for the deprecation warning I think
> I'd say something like "use .loc with labels, or .iloc with positions instead." I don't think we can really say which will be easier for the user in the error message, so just don't take a position.

Makes sense, updated.
thanks @jbrockmendel
Docstring fixes for PR06 errors
State: closed

This contains some small changes that resolve some of the PR06 errors from running `./scripts/validate_docstrings.py --errors=PR06`
- [x] xref #28724
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Docs

Comments:
Thanks @jchen2186
REF: make _setitem_with_indexer iloc-only
State: closed

some overlap with #31797.  

_setitem_with_indexer is used for positional indexers, so it is misleading for it to be in loc anyway.

_setitem_with_indexer is an absolute beast, will be easier to sort out when we dont have to worry about what class we're in.

The diff looks misleading.  All this does is
- move _setitem_with_indexer, _setitem_with_indexer_missing, _align_series, _align_frame from LocationIndexer to _iLocIndexer
- make `LocationIndexer.__setitem__` call `_has_valid_setitem_indexer` before calling _setitem_with_indexer instead of inside of it, so we get the right class's _has_valid_setitem_indexer
- dispatch to iloc if we are not already iloc
- update usages in core.frame

Labels:
name: Refactor
description: Internal refactoring of code
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves

Comments:
i'll trust you on the diff; needs a rebase
rebased+green
conflict, can you rebase
go ahead and merge on green.
Fixed mypy errors in pandas/tests/extension/json/test_json.py
State: closed

Part of #28926
- [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
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
Thanks @saturnfromtitan
CLN: disallow kind=None in _convert_slice_indexer
State: closed

The one place where _convert_slice_indexer is currently called without kind is within _setitem_with_indexer, which I've determined should be iloc-only so it is a no-op.  With that usage removed, we can be stricter about what gets passed.

Labels:
name: Refactor
description: Internal refactoring of code
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves

Comments:
sure
Test messages test integer
State: closed

- [ ] xref #30999
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] 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: Error Reporting
description: Incorrect or improved errors from pandas

Comments:
thanks
DOC: Use consistent casing in headers
State: closed

This is super nitty, but in the left side bar of the user guide the nullable Boolean data type section is the only one that uses all caps (other than Frequently Asked Questions, which possibly makes sense), so this might look a bit better.

https://pandas.pydata.org/pandas-docs/stable/user_guide/index.html#user-guide

Labels:
name: Docs

Comments:
sure
Test messages test period
State: closed

- [ ] xref #30999
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] 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: Period
description: Period data type

Comments:
thanks @RaisaDZ
correct redirections in doc/redirect.csv for rolling
State: closed

- [x] closes #31762 


Labels:
name: Docs

Comments:
@WillAyd I tried and found other rolling related links broken too. So, I have changed other rolling related redirections too.
Sounds good. Is the sphinx build generating warnings about these links perhaps? Wondering if there is a way to identify more
thanks @rushabh-v 
We also need new redirects from `/pandas.core.window.Rolling...` to `pandas.core.window.rolling.Rolling...` to fix the actual issue reported by @WillAyd (so reopened the issue)
Sure, will be opening a PR soon :)
Sorry @jorisvandenbossche I couldn't figure out what you did mean by,
>We also need new redirects from /pandas.core.window.Rolling... to pandas.core.window.rolling.Rolling... to fix the actual issue reported by @WillAyd 

Can you please elaborate a bit more?
@rushabh-v check the two links mentioned y @WillAyd in the original issue: https://github.com/pandas-dev/pandas/issues/31762 The first link should redirect to the second.
DOC: '10 minutes to pandas' - <TAB> completion section now show a con…
State: closed

…sistent block of suggestions. Solve issue #31526

- [x] closes #31526
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

I didn't run those listed tests above, however I doubt it will break something.

This PR only correct a *Typo* I found on documentation. I hope I'll be useful to improve this project.

Labels:
name: Docs

Comments:
@Cardosaum : First off, welcome to `pandas` 🎉 

I have a quick question about your changes.  Otherwise, would be good if someone else from @pandas-dev/pandas-core could review this, as I can't check this.
I corrected this error, @gfyoung . 

Thanks for spotting this!
thanks @Cardosaum
check parser_dates names in columns
State: closed

if column names are passed in parser_dates, make sure those
columns exist in dataframe.

- [x] closes #31251 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Error Reporting
description: Incorrect or improved errors from pandas
name: IO CSV
description: read_csv, to_csv

Comments:
Hi @MomIsBestFriend , I have fixed it. Could you check again. I'm not sure why CI is failing. How do I investigate.
I am restarting the CI, I can't see what's wrong
All checks passed, could you review and merge.
cc @gfyoung 
closing as duplicate of #32320
BUG: iloc setitem with 3d indexer not 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

also get the exceptions/messages in test_setitem_ndarray_3d to be maximally-specific

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

Comments:
thanks @jbrockmendel
CLN: some code cleanups in pandas/_libs/
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:
The only "big" changes I have done here, is taking this pattern:

```python

arr = ["a", "b", "c"]

for i in range(len(arr)):
    print(arr[i])
```

and making it to look like this:

```python
arr = ["a", "b", "c"]

for value in arr:
    print(value)
```
---

When it comes to python, I can't see why would one use the first pattern, but when it comes to Cython, I don't know if there's some sort of optimization, that the first pattern gives that the second one don't
> but when it comes to Cython, I don't know if there's some sort of optimization, that the first pattern gives that the second one don't

Then you will need to show that it gives similar results (eg show some timings of the changed code, or look at the generated C code).

For example, with a simple test (but I don't know if that is fully equivalent with the changes you did, this was just a quick test), I see a big overhead when iterating directly through the values of an array. Eg check

```
%%cython -a
import cython

import numpy as np
cimport numpy as np


@cython.wraparound(False)
@cython.boundscheck(False)
def sum1():
    cdef:
        np.int64_t[:] arr
        int n
        int val, total
    
    arr = np.random.randint(10, size=10000)
    
    n = len(arr)
    total = 0
    
    for i in range(n):
        val = arr[i]
        total += val
    
    return total
```

vs

```
%%cython -a
import cython

import numpy as np
cimport numpy as np


@cython.wraparound(False)
@cython.boundscheck(False)
def sum2():
    cdef:
        np.int64_t[:] arr
        np.int64_t val, total
    
    arr = np.random.randint(10, size=10000)
    
    n = len(arr)
    total = 0
    
    for val in arr:
        total += val
    
    return total
```

the second version is much slower (and with the annotation in the notebook you clearly see a lot of python interaction). But I am no cython expert, so I might also have done something wrong.
@MomIsBestFriend i'd advocate reverting the non-easy parts of this and getting the clear improvements in
@jbrockmendel NP, I will do it over the weekend as soon as I have time.
@MomIsBestFriend can you rebase and ping on green
ping @jbrockmendel 
thanks @MomIsBestFriend
CI Fix w/o tm helper
State: closed

Alternate to #31791

Labels:

Comments:
LGTM
TBH I am ```-1``` on that, as we are trying to get rid of all the ```bare pytest raises```.
maybe the name of ```external_error_raised``` is not good, and needs a different (and better) name.

As far as I know we are planning to have a CI check for no ```bare pytest raises```, obviously there are some exceptions such as an external error message, we need to agree on how to mark ```bare pytest raises``` that are bare on purpose. 

A conversion on that topic has been started at https://github.com/pandas-dev/pandas/issues/30999#issuecomment-574535402
@momisbestfriend the problem is that helper isn’t backported so can’t be used to fix CI
> @MomIsBestFriend the problem is that helper isn’t backported so can’t be used to fix CI

we can simply backport the helper, no?
Sure that would be an alternative
Looks like the CI failures may have resolved themselves on the 1.0.x branch, so I guess no need to do this. However, should probably revert original PR then - @jbrockmendel do you agree? Something you have time to tackle?
> Something you have time to tackle?

I'm stretched pretty thin ATM.  Won't hitting the green button here get the job done?
Closing in favor of #31931
Added pd.NA to nulls_fixture
State: closed

@jorisvandenbossche 

I think we should do this. Simply xfailed issues up for discussion on how to resolve


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

Comments:
Cool! Yes, I agree it's a good idea to add it, and skip for now where it is not yet supported
should this use the dynamic-xfail pattern we've used elsewhere?

+1 on the idea
Opened issues where I think applicable and updated comments accordingly
Any other feedback here?
this is ok

Assume u have opened an issue for the xfail u r adding
Noted in tests - they are #31884 #31883 and #31881
Thanks all for feedback
CLN: assorted indexing-related cleanups
State: closed

- _setitem_with_indexer isnt consistent about whether or not it returns anything, make it always-None
- avoid using private loc method from DataFrame


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

Comments:
does this change anything user facing? (no?)
> does this change anything user facing? (no?)

Correct, nothing user-facing.
Thanks!
ENH: add Series.info
State: closed

- [x] initial draft to close #5167
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

screenshot of the example
![image](https://user-images.githubusercontent.com/33491632/74069703-f1754b80-49f6-11ea-8c9e-8ec4e16166f1.png)

------

Note to self: this change from `ba72b59b44f15076877e784e16cbc66aa03e0adc` breaks the tests

```diff
-    # groupby dtype.name to collect e.g. Categorical columns
-    counts = data.dtypes.value_counts().groupby(lambda x: x.name).sum()
+    counts = data._data.get_dtype_counts()
```

----

### Updated

Currently, looks like this:

![image](https://user-images.githubusercontent.com/33491632/86509914-7a948100-bde3-11ea-84fb-907df7a2a40c.png)


Labels:
name: API Design
name: Series
description: Series data structure

Comments:
Some screenshots
![image](https://user-images.githubusercontent.com/33491632/74101674-4c608d00-4b34-11ea-9e65-8e9af8263c5d.png)
![image](https://user-images.githubusercontent.com/33491632/74101678-597d7c00-4b34-11ea-84f6-56a1b5d0e28f.png)
![image](https://user-images.githubusercontent.com/33491632/74101661-2fc45500-4b34-11ea-8d54-a06ebda996af.png)
![image](https://user-images.githubusercontent.com/33491632/74101666-3bb01700-4b34-11ea-8736-55e31d90bbef.png)

Hello @MarcoGorelli! 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-09-19 16:26:44 UTC
Thanks @WillAyd for your review, have updated
Screenshots of docs:

![image](https://user-images.githubusercontent.com/33491632/74838860-3749e200-531c-11ea-9117-d570ec14b2d9.png)
![image](https://user-images.githubusercontent.com/33491632/74838894-4466d100-531c-11ea-9b85-7bfd003c2eb2.png)
![image](https://user-images.githubusercontent.com/33491632/74838917-50529300-531c-11ea-8c74-ed3bac788d25.png)

@MarcoGorelli large PRs are hard to merge. but if you merge master can take a look again. ideally you can factor out pieces of this to share code and it will be easier to get in.
@jreback have merged / factored some pieces out. Is this OK to review or should I factor some more pieces out?
I've pushed a new attempt at this. I've factored out some pieces of `_verbose_repr` to avoid code duplication, although their arguments lists and return values have become rather long.
@MarcoGorelli can you fix up the PR here? Someone will take a look again thereafter
> @MarcoGorelli can you fix up the PR here? Someone will take a look again thereafter

@WillAyd Sure, have given another go at it, now using a namedtuple to group together some similar variables
(closing as @ivanovmg will take it over)
ENH: add from_dummies
State: closed

Just a first draft

Will close #8745 

Screenshot of examples

![image](https://user-images.githubusercontent.com/33491632/74546261-3989f600-4f42-11ea-98e0-14c8ae4658ff.png)


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

Comments:
Hello @MarcoGorelli! 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-18 10:00:29 UTC
@WillAyd have made some updates. See screenshots in original post for examples.

Have removed `fill_first`. Haven't added equivalents of `dummy_na` or `sparse` - would you prefer to get a bare-bones implementation out first and add those in future PRs, or should I add them now?
@WillAyd thanks for your review, have updated
@MarcoGorelli did you see this impl: https://github.com/pandas-dev/pandas/issues/8745#issuecomment-630924929.

I also don't see why we would ever return anyting but a Series / Categorical here. can you elaborate?
> @MarcoGorelli did you see this impl: [#8745 (comment)](https://github.com/pandas-dev/pandas/issues/8745#issuecomment-630924929).
> 
> I also don't see why we would ever return anyting but a Series / Categorical here. can you elaborate?

I hadn't, thanks for pointing me towards it! @clbarnes that looks good, are you interested in taking over this issue with your implementation?
I can do if you'd prefer that strategy - will need to have a think about the requirements, e.g. what kind of incorrect inputs should it try to gracefully handle, handling masked arrays and NA and so on. That's probably discussion for the issue though.

I personally err on the side of being less permissive than pandas is generally so it may end up being pretty constrained, if that's OK with the maintainers.
> I can do if you'd prefer that strategy

Sure, go ahead! I'll close this PR then, though please let me know if for whatever reason you chose not to work on this
BUG: Avoid casting Int to object in Categorical.from_codes
State: closed

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


Labels:
name: Categorical
description: Categorical Data Type
name: Constructors
description: Series/DataFrame/Index/pd.array Constructors

Comments:
Can you merge master? CI failure should be resolved on master
thanks @dsaxton 
@MeeseeksDev backport to 1.0.x
Some code cleanups
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:
Seems to have introduced a few test failures? Generally looks good
Thanks @MomIsBestFriend
CI: fix feather test
State: closed


Labels:
name: CI
description: Continuous Integration

Comments:
updated to use external_error_raised.  Do we want to check anything about the message being reasonable?
> updated to use external_error_raised. Do we want to check anything about the message being reasonable?

I don't think we do, as this is an external error message.
But I would like to hear others as well.
Thanks @jbrockmendel yea I don't think we need to test for 8 different messages!
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 6d3cc14fc52685511329cedf1e11d15651ac6a8e
```

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

```
$ git commit -am 'Backport PR #31791: CI: fix feather test'
```

4. Push to a named branch :

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

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

> "Backport PR #31791 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).
                
Hmm I guess that helper isn't available on the 1.0.x branch
why exactly was this merged @WillAyd @jbrockmendel @jorisvandenbossche  was this fixing a CI issue?

it is not about testing for the error messages, but the fact that the error messages are *different* that is extremely troubling. This is de-facto removing legitimate tests.

Yea looks to have failed CI
I think alternately would have to add a 9th error message to test for with current design; maybe should break up and test error message specifically by reader?
msg4 doesn't look like an external message. should external_error_raised fail this? @MomIsBestFriend 
not at a computer to check though. so could be external. just looks like one of ours.
> msg4 doesn't look like an external message. 

Yes, I think that is the only one raised by ourselves (it's "path_or_buf needs to be a string file path or file-like"). But it's actually also a wrong error message we were testing .. as what is passed *is* a string file path)
> msg4 doesn't look like an external message. should external_error_raised fail this? @MomIsBestFriend

@simonjayhawkins ```tm.external_error_raised``` is an alias for 
```pytest.raises(<An exception class>, match=None)```.

It does not check for whether or not this is an actual external error message.
TST: Making use of tm.external_error_raised
State: closed

- [ ] closes #xxxx
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

After #31130 got merged, I can do what I couldn't do [here](https://github.com/pandas-dev/pandas/pull/30998#discussion_r368177775)



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

Comments:
I think this is confusing to say `external_error_raised` for something in internals
Can you alternately not just add the message?
> I think this is confusing to say `external_error_raised` for something in internals

@WillAyd I am with you on that, still it's raising an external error message, do you suggest moving it to somewhere else?
> Can you alternately not just add the message?

@WillAyd I really prefer not to, in #30999 we are fixing all the bare ```pytest raises```, and in order to silent the test case in #30755 we have to pass in the "match" argument, we could just pass in the ```match=None``` but as you said [here](https://github.com/pandas-dev/pandas/pull/30998#discussion_r368177775) this is not very clear.
I don't have a problem with `match=None` per se - where you linked my comment I was just asking what the purpose of it was

As mentioned I don't think this adds any value and if anything is confusing, but let's see what others think
Closing as I don't think this is moving in the right direction, but thanks in any case @MomIsBestFriend !