BrandonFors/pandas-github-issues huggingface dataset:
REF: use _get_string_slice in PeriodIndex.get_value
State: closed

Most of _get_string_slice is duplicated inside PeriodIndex.get_value, which likely explains why get_string_slice has almost no test [coverage](https://codecov.io/gh/pandas-dev/pandas/src/master/pandas/core/indexes/period.py#L715).  This also brings PeriodIndex.get_value into closer alignment with DatetimeIndex.get_value, so we may be able to share code a few steps from here.



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

Comments:
needs a rebase, this overalaps with #31096 ?
> needs a rebase, this overalaps with #31096 ?

rebased+green.  the diff overlaps so 31096 will probably need to be rebased, but they are logically independent.
Split out JSON Date Converters
State: closed

This file is pretty large so trying to make more modular. This separates out conversion routines required for getting from the Python space to JSON, but which don't actually serialize the data


Labels:
name: IO JSON
description: read_json, to_json, json_normalize
name: Clean

Comments:
Hello @WillAyd! 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-20 17:16:31 UTC
seems like a good refactor.

Out of curiosity, what is the big picture gameplan for the json code?
Just trying to make more readable / scalable and learn more about C
LGTM cc @jreback
can you rebase
rebased and green
lgtm. thanks @WillAyd
DOC: Fix HDF5 complevel and complib formatting
State: closed

Fixes formatting of these params.

- [x] closes #31052
- [ ] tests added / passed (N/A)
- [ ] passes `black pandas` (N/A)
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff` (N/A)
- [ ] whatsnew entry (N/A)

N/A as this PR only updates an rst file for the docs.

Labels:
name: Docs

Comments:
can you post a screen shot of the rendered
> can you post a screen shot of the rendered

My experience rst eye says this is proper formatting for a list ;) So it should be good.
Thanks @kylekeppler !
DOC: Add missing docstrings
State: closed

Added docstrings:
```
pandas.Index.has_duplicates
pandas.Index.is_all_dates
pandas.Index.name
pandas.Index.is_boolean
pandas.Index.is_floating
pandas.Index.is_integer
pandas.Index.is_interval
pandas.Index.is_mixed
pandas.Index.is_numeric
pandas.Index.is_object
```

There are some other docstrings left - I couldn't find the docstrings below in the code:
```
None:None:GL08:pandas.Index.names:The object does not have a docstring
None:None:GL08:pandas.Index.empty:The object does not have a docstring
```

I don't really understand what the function does so I'm leaving the docstring empty for now:
```
pandas/pandas/core/indexes/base.py:639:GL08:pandas.Index.view:The object does not have a docstring
```

cc @datapythonista

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Docs

Comments:
If you have a look at the CI, there are couple of errors in the docstrings that are making it fail: https://github.com/pandas-dev/pandas/pull/31047/checks?check_run_id=391576648#step:10:24
@galuhsahid Thanks a lot!
TST: Disallow bare pytest.raises
State: closed

- [ ] ref #30999 
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

This pull request is to add appropriate match arguments to bare pytest.raises listed in the comment https://github.com/pandas-dev/pandas/issues/30999#issuecomment-574174389 as described by the referenced issue.

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

Comments:
I would like to verify that I am approaching the issue correctly on this single test before I move on to the rest of them.  My error message is based on the three formats of error messages that are currently raised by this test.  Is this an acceptable match argument?

Another question, running "black pandas" modifies many files in the code base, should I just ignore this and only run it on files I modify or are my settings improperly configured?

Could you comment on this @MomIsBestFriend ?  Thanks.

EDIT: Another question, there seems to be many failed tests in pytest locally even before I change the code.  Is my setup incorrect or is this expected?
Thanks @gdex1 for the PR. The approach looks about right.

This one may be a bit tricky though. The test is parameterized and it appears that the user-facing error messages are not consistent. The test failures point to not all the combinations being represented in `msg`. (such as `must be str, not DatetimeArray`)

> Another question, running "black pandas" modifies many files in the code base, should I just ignore this and only run it on files I modify or are my settings improperly configured?

Weird, have you set your virtual environment / conda environment as described in the [pandas contributing guide](https://dev.pandas.io/docs/development/contributing.html#creating-a-development-environment) ?

> EDIT: Another question, there seems to be many failed tests in pytest locally even before I change the code. Is my setup incorrect or is this expected?

Might be also related to the same reason that ```Black``` is changing a lot of files, have you compiled all the extensions?


Thanks @gdex1
BUG: AssertionError on Series.append(DataFrame) fix  #30975 
State: closed

- [x] closes #30975 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Series.append() does not throw AssertionError anymore. This fix performs a precheck before proceeding with concatenation. Tests have passed. Whatsnew entry added.
Thanks!

Labels:
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Regression
description: Functionality that used to work in a prior pandas version

Comments:
Hello @hvardhan20! 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 06:02:52 UTC
This seems fine for backport.

What's the plan for 1.0.1 / 1.1? For this to raise a TypeError? Do we have an issue for that?
thanks @hvardhan20 

can you open an issue as @TomAugspurger suggest https://github.com/pandas-dev/pandas/pull/31036#issuecomment-577248825
TST: Split test_offsets.py - Added fixtures for date offsets and business date offsets
State: closed

In context of #30194 and #31031
Second part with added (bussiness-)date offsets fixtures
- [ ] closes #27085
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


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

Comments:
@Raalsky still active? Can you address above comments?
Revert "DEPR: is_copy arg of take (#30615)"
State: closed

This reverts commit 7796be6e2c548de0be16e6adf08f5fca2ab9b4bc.

See https://github.com/pandas-dev/pandas/issues/27357#issuecomment-571639242 and https://github.com/pandas-dev/pandas/pull/30615#issuecomment-571637550

Labels:

Comments:
> i would rather fix take later if necessary, rather that redo this PR and fix take later with necessary

If we want to fix `take` later, why should we remove that behaviour of `take` now which we want to preserve later? (the fact that `take` is an actual copy)

BTW, I can also try to do a PR that actually fixes `take`. This is basically https://github.com/pandas-dev/pandas/pull/30784, but in addition we would need to keep a private version of `take` that does the `set_is_copy` thing and ensure to use that private version where appropriate (i.e. mainly in the indexing code). 
But then I first like to have some agreement on that general idea before putting time in it.
> > i would rather fix take later if necessary, rather that redo this PR and fix take later with necessary
> 
> If we want to fix `take` later, why should we remove that behaviour of `take` now which we want to preserve later? (the fact that `take` is an actual copy)
> 
> BTW, I can also try to do a PR that actually fixes `take`. This is basically #30784, but in addition we would need to keep a private version of `take` that does the `set_is_copy` thing and ensure to use that private version where appropriate (i.e. mainly in the indexing code).
> But then I first like to have some agreement on that general idea before putting time in it.

that would be preferable

when private and public take were consolidated a while back this must have been broken

also include the tests for no setting with copy warning

thanks
@jreback ok, did what is described above in https://github.com/pandas-dev/pandas/pull/30784

So closing this PR then
TST: Split test_offsets.py - Test_offsets moved to test_date_offsets
State: closed

In context of [https://github.com/pandas-dev/pandas/pull/30194](30194)
First part with move test_offsets to test_date_offsets
- [ ] closes #27085
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:

Comments:
Are you planning to split this module up? Just reading back through the original PR I think that is the request; not just to rename it
Closing as I think this is stale but @Raalsky ping if you'd like to pick back up and can address comments
Disallow bare pytest.raises
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

https://github.com/pandas-dev/pandas/issues/30999

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Code Style
description: Code style, linting, code_checks

Comments:
Thanks @DylanBrdt for the PR. In future you should create a new branch on your local fork when making a PR request. see https://dev.pandas.io/docs/development/contributing.html#creating-a-branch
Hi @DylanBrdt - are you still interested in working on this?
@MarcoGorelli I appreciate your consideration of asking me. Unfortunately, I am still learning OSS and want to learn the basics of git before I attempt any more issues.
Thank you.
OK. closing for now.
REF: DatetimeIndex.get_loc
State: closed

xref #30874 which does similar cleanup for TimedeltaIndex, #31021 for PeriodIndex

Do all parsing/casting/conversion before call to self._engine or super() methods  (xref #30948)

Needs test for preventing integers and Timedeltas through incorrectly.

Some of the casting may be simplified pending #30994.

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

Comments:
thanks @jbrockmendel 
excellent.  i think that just leaves #30874 as the TDI analogue to this, then we start in on de-duplication and optimization
REF: PeriodIndex.get_loc
State: closed

xref #30874 which does similar cleanup for TimedeltaIndex.

Do all parsing/casting/conversion before call to self._engine or super() methods (xref #30948)

Fix+test integers incorrectly getting through.



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

Comments:
thanks @jbrockmendel
REG: restore format_type attr
State: closed

- [x] closes #30962

cc @TomAugspurger 

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

Comments:
Thanks. Two questions:

1. Thoughts on the need for a simple test? I worry a bit about a comment being overlooked in a future refactor clearing stuff out / looking at "dead" code.
2. Do we want this API going forward? Clearly, we don't have a need for it. Is there an alternative we should deprecate in favor of? 
> 1. Thoughts on the need for a simple test?

code sample in issue as test in pandas\tests\test_downstream.py
wrong button!
@TomAugspurger suggestions for a snippet to test?
Nothing fancy. Just an asset that hdf.get_storer(key).format_type is the expected value for that format
Is the purpose of this field documented anywhere? If not can we add a comment?
Pushed a test.
> Is the purpose of this field documented anywhere? If not can we add a comment?

IMO we can keep PR to just fixing the regression.
Thanks @jbrockmendel.
BUG: pivot_table with multi-index columns only fails
State: closed

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


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

Comments:
very nice @charlesdong1991 

thanks!
DOC: Moved PANDAS_TESTING_MODE tip to .travis.yml (#30694)
State: closed

Added comments for travis.yml for PANDAS_TESTING_MODE from tips and tricks section of wiki

- [X] xref #30964
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Docs

Comments:
thanks @datapythonista i don't believe this closes the referenced issue.
Agree, we need to see if there is anything else in that wiki we want to keep.
DOC: removed wrong value of `pd.NA ** 0`
State: closed

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


Labels:
name: Docs

Comments:
Nice! Seems like a good decision to put this inside an `ipython` block to avoid similar mistakes in the future
I think I prefer this as a table so that it is formatted nicely in the docs. Curious to hear what others think.
Let's go with that then.
@TomAugspurger should the auto-backport kick-in moreless straight-away
Oh, whoops, this was merged to the wrong branch. PRs should go to master.
So, I think I will revert this, make a new PR to master, and then use the bot to backport it... Probably not necessary, but let's do it to not mess up history / branching.
OK, reverted on 1.0.x, and made https://github.com/pandas-dev/pandas/issues/31003 to master (which will be backported).
I thought we need to work on the development branch, and bit on master. Hmmm. How axtually do you manage your Branch's in pandas?
> I thought we need to work on the development branch, and bit on master. Hmmm. How axtually do you manage your Branch's in pandas?

Everything is merged in master, and the appropriate commits for 1.0.0 are backported into the 1.0.x branch. So in practice, almost all PRs should target the master branch.
TST: bare pytest raises
State: closed

- [x] ref #30999 
- [ ] 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: Clean

Comments:
Thanks @MomIsBestFriend
TST: Insert 'match' to bare pytest raises
State: closed

- [x] ref #30999
- [ ] 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: Clean

Comments:
@simonjayhawkins Do you mind if I open a master issue, instead of #23922 ?
There's a lot of noise there about how to find a bare pytest raises, I have a test case and I can point at the line number at each file with a bare pytest raise, and will make a list of occurrences (similar to #29886)

---

Do you think it's a good idea?
> Do you think it's a good idea?

The format of #29886 worked well for community engagement. so OK with opening a new issue to supersede #23922.
pending addressing comments from @WillAyd, LGTM
Thanks @MomIsBestFriend
STY: Whitespaces placed at the beginning instead at the end of a line
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: Code Style
description: Code style, linting, code_checks

Comments:
Thanks @MomIsBestFriend
STY: concat strings
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: Code Style
description: Code style, linting, code_checks

Comments:
Will release the test case for thees soon, the test case for thees still have many (so many) bugs.

But when I think about it again, isn't [Black](https://github.com/psf/black) spouse to do this thing for us?
I don't talk about actually concatenating the strings, they made it very clear that they're not going to do so [here](https://github.com/psf/black/issues/1051) (that's why we have #30467).
I mean, just putting them together (the strings), [Black](https://github.com/psf/black) is already concatenating the strings with "easy" strings like:

```python
msg = (
     "foo "
     "bar "
     "baz "
)
```

to 

```python
msg = ("foo " "bar " "baz")
```

But when it comes to strings with more characters it won't change them.

For example,  [Black](https://github.com/psf/black)  is __not__ going to change this (even if the line length is below 88) :

```python
def func():
    msg = (
        "foo foo foo foo foo foo foo foo foo "
        "bar bar bar bar bar bar bar bar bar "
        "baz"
    )
```

to this:

```python
def func():
    msg = (
        "foo foo foo foo foo foo foo foo foo "
        "bar bar bar bar bar bar bar bar bar " "baz"
    )
```

---

Is there a feature of [Black](https://github.com/psf/black) we are missing?

@MomIsBestFriend I don't want to speak for black but not sure it's that big of a deal. Sometimes readability can be improved by breaking at particular points in the string rather than always trying to pad to 88 characters

OK to continue along with f-string replacement but I don't think its a goal to always pad strings to their fullest length on a line, so maybe not worth investing a ton of time into
> @MomIsBestFriend I don't want to speak for black but not sure it's that big of a deal. Sometimes readability can be improved by breaking at particular points in the string rather than always trying to pad to 88 characters
> 
> OK to continue along with f-string replacement but I don't think its a goal to always pad strings to their fullest length on a line, so maybe not worth investing a ton of time into

@WillAyd You have a very good point.
Added clang inline helper
State: closed

gcc on macOS I think executes clang more often than not, and it appears we didn't have a inline helper defined for that. This is ported from `CYTHON_INLINE` which essentially does the same in cythonized files, save the static declaration

This might add a bit of build time to macOS (I think partially explains why it's a few minutes faster than the rest) but makes it more similar in performance to the others

Running the full benchmark suite to see where this may help

Labels:
name: Performance
description: Memory or execution speed performance
name: Internals
description: Related to non-user accessible pandas implementation

Comments:
makes sense, looking forward to the perf stats
Doesn't clang define `__GNUC__`?
From what I understand clang defines it but the semantics of the `inline` keyword are different
Have you checked whether binaries produced before this patch and with this patch are different? I dont' see why `__attribute__ ((__unused__))` would make a difference to the compiler's choice to inline a function. I think it suppressed some warnings.
I tried running the entire benchmark suite but got stuck at 30% somewhere over night, so re-ran just for indexing this morning. Here are the results:

```sh
       before           after         ratio
     [8ff2ebd9]       [77a96005]
     <axis-cleanup>       <clang-inline>
-      5.05±0.2ms      4.31±0.03ms     0.85  indexing_engines.ObjectEngineIndexing.time_get_loc('non_monotonic')
-         179±2μs          151±7μs     0.85  indexing.NumericSeriesIndexing.time_getitem_scalar(<class 'pandas.core.indexes.numeric.Int64Index'>, 'nonunique_monotonic_inc')
-        24.4±1ms       17.1±0.5ms     0.70  indexing.NumericSeriesIndexing.time_loc_scalar(<class 'pandas.core.indexes.numeric.UInt64Index'>, 'nonunique_monotonic_inc')
-        44.4±3ms         26.0±2ms     0.59  indexing.InsertColumns.time_assign_with_setitem

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.
```

So I think a measurable boost in some places
any firm conclusions about the perf impact?
Yea looks to give a boost to indexing operations. I didn't run the entire test suite because this inline helper is already generated by Cython so already used in anything but our custom C code
Planning to merge at the end of the day tomorrow unless any other comments @pandas-dev/pandas-core 
That is part of this, but the MSVC inline could become `static inline`.  

Do any of these compilers have different behavior for vendor-specific keywords rather than C99 `static inline`?
Should 

https://github.com/pandas-dev/pandas/pull/30990/files#diff-b3aa9f22c2ca2e89ba79863c5701aafcL18

be first?

GNUC says: "If you are writing a header file to be included in ISO C90 programs, write __inline__ instead of inline. See Alternate Keywords."
Not sure, but I don't know if worth straying from what Cython generates for this in any case
i guess this can't hurt.
Update _json.py
State: closed

Without this change, use of the chunksize argument in read_json causes the following error:

`TypeError: initial_value must be str or None, not bytes`

- [x] closes #28906
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:

Comments:
I'm not sure we should make this change given JSON is a text-only format by definition. Is this not something that can be controlled on the user end?
> I'm not sure we should make this change given JSON is a text-only format by definition. Is this not something that can be controlled on the user end?

I think I probably shouldn't have referenced that issue with my pull request, but it did seem related...

In my case, I wasn't trying to process binary data, but rather had a huge text file where each line was a json object. If I specified the chunksize, I got the error I mentioned in the issue, specifically:

`TypeError: initial_value must be str or None, not bytes`

Also it seems my patch isn't working, so I'll have to tweak it and retest...
Hello @copeland3300! 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-13 23:48:44 UTC
Well it appears I'm worse at python than I thought, so I'll close this pull, but hopefully this will lead to some resolution in the future.
@copeland3300 I don't think it's so much about "being bad at Python" but more about getting used to the contributing process - have you read through the [contributing guide](https://pandas.pydata.org/pandas-docs/stable/development/contributing.html)?

As an aside, perhaps you meant `not isinstance(data, str)` instead of `type(data) != "str"`?