BrandonFors/pandas-github-issues huggingface dataset:
Bug 29764 groupby loses index name sometimes
State: closed

- [x] closes #29764
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry´

Some group by functions lost the column index name. For the functions running into ```_get_cythonized_result``` in groupby.py, the index name was just ignored when defining the column names. So the following functions ```_wrap_aggregated_output``` and ```_wrap_transformed_output``` in the class DataFrameGroupBy had not acces to this information, because it was already lost there. I collected the information beforehand and defined the Index Name accordingly.

We could refactor both methods a bit at a later stage, because at it is right now (and was before) the first few lines are duplicates.

Labels:
name: Bug
name: Groupby

Comments:
can you merge master and will have a look
if u can merge master and update will look
@phofl can you move release note to 1.2 and merge upstream/master to resolve conflict
Merged master and moved whats new entry. I hope, that I will be able to change the functions as requested by jereback in the next few days. I have not that much time currently
Issue was fixed for sum and any with c9144ca54dcc924995acae3d9dcb890a5802d7c0. I close this PR and open an new one for the rest
REF: push concat logic out of internals and into concat_compat
State: closed

This changes some empty-array behavior for concat_compat to match the behavior of pd.concat(list_of_series), see new test.

The follow-up to this basically gets concat out of internals altogether.

Labels:
name: Refactor
description: Internal refactoring of code
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode

Comments:
thanks
Docstring for show_versions in master
State: closed

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


Labels:
name: Docs

Comments:
Connected with issue #33044 and PR #33073.
As @MomIsBestFriend mentions https://github.com/pandas-dev/pandas/pull/33073#issuecomment-605607698, you can just update #33073 and merge master into _that_ branch.

Let's continue development in #33073 and address @jreback's comments there
ENH/VIZ: Allowing `s` parameter of scatter plots to be a column name
State: closed

- [x] closes #32904 
- [x] tests added
- [x] tests passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

This PR is a continuation of #32937 (made an error when pulling changes from the master).

I had a more elaborate decision tree for what to do with a passed size variable `s`, here https://github.com/pandas-dev/pandas/pull/32937, but in the end decided to go with the simpler version since the other checks are redundant (they would contain `s=s` or `pass`).

Labels:
name: Enhancement
name: Visualization
description: plotting

Comments:
Thanks @SultanOrazbayev. Congrats on your first PR to pandas!
Thank you very much for your help! Glad it worked out!
Fix mixup between mean and median
State: closed

Text said "median", but code uses "mean". Make text match the code

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Docs

Comments:
thanks @oefe
Add test for #32108 (error with groupby on series with period index)
State: closed

- [ ] closes #32108


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Groupby
name: Period
description: Period data type

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



There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-06-24 20:14:10 UTC
I've done the corrections, all tests are passing now, sorry for the delay
@daxid thanks for working on this. seems to have been lost in the shuffle. can you merge master to trigger ci.
> @daxid thanks for working on this. seems to have been lost in the shuffle. can you merge master to trigger ci.

done
can you merge master
> can you merge master

done
Done once again
Is that version good for you ?
it appears that https://github.com/pandas-dev/pandas/pull/33105#discussion_r440134823 has not been addressed period_serie -> period_series otherwise lgtm. Thanks @daxid 
> it appears that [#33105 (comment)](https://github.com/pandas-dev/pandas/pull/33105#discussion_r440134823) has not been addressed period_serie -> period_series otherwise lgtm. Thanks @daxid

I am not sure to understand you request, do you want us to add a `name=` parameter  in the `period_serie` construction or do you want us to change the name `period_serie` to something else, maybe `month_serie` ?
> > it appears that [#33105 (comment)](https://github.com/pandas-dev/pandas/pull/33105#discussion_r440134823) has not been addressed period_serie -> period_series otherwise lgtm. Thanks @daxid
> 
> I am not sure to understand you request, do you want us to add a `name=` parameter in the `period_serie` construction or do you want us to change the name `period_serie` to something else, maybe `month_serie` ?

Pretty sure the request is to rename `period_serie` to `period_series`
> Pretty sure the request is to rename `period_serie` to `period_series`

Thanks did now saw the `s` so I wasn't understing this request, should be good now

thanks @daxid
BUG: ensure consistent behavior of Index.is_all_dates (#19204)
State: closed

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

Labels:
name: Index
description: Related to the Index class or subclasses

Comments:
@addammy can you rebase
> @addammy can you rebase

Done
@addammy can you try to get the CI passing
can you merge master; ping on green.
closing as stale
if u want to continue pls ping
Check error message for raised exception
State: closed

- [x] xref #30999
- [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

Comments:
@MomIsBestFriend Please review.
@MomIsBestFriend Green.
>     * [x]  closes #30999
> 
>     * [x]  tests added / passed
> 
>     * [x]  passes `black pandas`
> 
>     * [x]  passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
> 
>     * [ ]  whatsnew entry

@sumanau7 Can you please change the word "closes" to the word "xref"?
@MomIsBestFriend Green.
thanks @sumanau7
PERF: fix performance regression in memory_usage(deep=True) for object dtype
State: closed

- [x] closes #33012  
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

The pull request is to update lib.memory_usage_of_objects from taking self.arrays to self._values. An ASV included to benchmark with and without object-dtype columns.

Before:
![Screen Shot 2020-03-28 at 11 57 22 AM](https://user-images.githubusercontent.com/33635204/77827342-6efe3380-70eb-11ea-9f32-6fd835cca76f.png)

After:
![Screen Shot 2020-03-28 at 11 56 42 AM](https://user-images.githubusercontent.com/33635204/77827344-71608d80-70eb-11ea-9745-e8754f6aebf2.png)

Labels:
name: Performance
description: Memory or execution speed performance

Comments:
@jbrockmendel thoughts?
LGTM.  @jorisvandenbossche has stronger opinions than i do on when to use `.array`
@jbrockmendel This is not about an opinion on when to use `.array` or not, this is about a cython routine needing a numpy array and not an EA. 
(so the alternative to `_values` would be `extract_array(... , extract_numpy=True)`, but since we already know we are object dtype here, it should be fine to use `_values`)
@neilkg Can you please merge master?
This should solve the failing test.

---

https://dev.pandas.io/docs/development/contributing.html#updating-your-pull-request
> @neilkg Can you please merge master?
> This should solve the failing test.

@MomIsBestFriend done
Thanks @neilkg! 
this was tagged for 1.04? hmm

not sure if we are doing that but no harm
i guess (though the release note will need to be moved)
DOC iris.csv file has moved
State: closed

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


Labels:
name: Docs

Comments:
/azp run
<samp>
Azure Pipelines successfully started running 1 pipeline(s).<br>

</samp>
Thanks @benabel 
Resolves https://github.com/pandas-dev/pandas/issues/33428
TST: GroupBy(..., as_index=True).agg() drops index 
State: closed

- [x] closes #28641
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

While searching for related issue for my other pull request, I found that this issue must have been fixed with another commit in the past (don't know when this issue was fixed).

I added a test to ensure that this error won't know occur in the future.

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

Comments:
Mind merging master? Failing test should be fixed 
Merge master
looks fine, can you merge master and ping on green.
thanks @phofl
TEST: Add test for #22541 to ensure that error does not occur again
State: closed

- [x] closes #22541
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

While searching for related issue for my other pull request, I found that this issue must have been fixed with another commit in the past (don't know when this issue was fixed).

I added a test to ensure that this error won't know occur in the future.

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

Comments:
thanks @phofl
TYP: enforce tighter inputs on SingleBlockManager
State: closed

there is currently only one place where we pass a list of indexes instead of a single index, so just fixed that and were good to go.

Labels:
name: Internals
description: Related to non-user accessible pandas implementation
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
lgtm. rebase and @jorisvandenbossche has some comments, ping on green.
@jorisvandenbossche i think ive addressed your comments, can you confirm?
Yes, thanks.

One more thing, though. For `Block`, we actually deprecated `fastpath` first instead of directly removing (only the passing of the keyword, I mean, not any functionality, https://github.com/pandas-dev/pandas/pull/19265). I assume SingleBlockManager will be used less in practice, but it's also not much work to still accept fastpath and raise a warning for it for now.
> deprecated `fastpath` first instead of directly removing

Sure
Thanks!
ENH: Add index option to_markdown()
State: closed

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


Labels:
name: Enhancement
name: IO Data
description: IO issues that don't fit into a more specific label

Comments:
@jreback apparently, the ```pytest.warns``` in my test is an unwanted pattern. Should i remove that part of code and ignore the testcase when ```index``` and ```showindex``` is both set?
@quangngd can you merge master.
@quangngd can you update as @TomAugspurger suggests (will need to change the test as well)
How tests should handle warnings?
> How tests should handle warnings?

Sure! There is a note on the contributing guide here: https://pandas.pydata.org/docs/dev/development/contributing.html#testing-warnings

Can you merge master + get a green CI?
@alimcmaster1 i don't know what to do with the codecov/patch fail
can you merge master and address comments
thanks @quangngd
BUG: Don't cast nullable Boolean to float in groupby
State: closed

- [x] closes #33071
- [x] closes #32194
- [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: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Regression
description: Functionality that used to work in a prior pandas version
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
Thanks @dsaxton for the PR. This also fixes the regression in #32194. can you add test for that case and also test for IntDtype, see https://github.com/pandas-dev/pandas/issues/33071#issuecomment-605441321
I've marked this 1.0.4 for now.
> Thanks @dsaxton for the PR. This also fixes the regression in #32194. can you add test for that case and also test for IntDtype, see [#33071 (comment)](https://github.com/pandas-dev/pandas/issues/33071#issuecomment-605441321)

@simonjayhawkins Added some new tests. Should I move the release note to 1.0.4 (and add something for the `agg(dict)` case)?
> Added some new tests.

thanks

> Should I move the release note to 1.0.4

not sure if a 1.0.4 is planned. @TomAugspurger ?
I’m not sure either. 

> On Mar 28, 2020, at 09:44, Simon Hawkins <notifications@github.com> wrote:
> 
> ﻿
> Added some new tests.
> 
> thanks
> 
> Should I move the release note to 1.0.4
> 
> not sure if a 1.0.4 is planned. @TomAugspurger ?
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or unsubscribe.

we are not tagging for 1.04
it’s would take quite a bit to backport
@dsaxton this looks good, can you merge master and ping on green.
Thanks @dsaxton
BUG: Groupby lost index, when one of the agg keys had no function all…
State: closed

…ocated

- [x] closes #32580
- [x] closes #33545 (is a duplicate of 32580)
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

The issue was the concatenation with an empty DataFrame and the result of the min function. This resulted in the lost index. 

I changed the input for the concatenation, so that only non empty DataFrames would be concatenated. We have to catch the case, that all DataFrames are empty, because this would result in an error.

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

Comments:
I am not sure I agree with supporting this case. what does an empty list in an aggregation even mean?
> 
> 
> I am not sure I agree with supporting this case. what does an empty list in an aggregation even mean?

Should the calculation fail as a whole in this case?
Alternatively, we could ignore the empty list as a whole in the aggregate function
will look at this again - might be ok with your soln 
can you merge master and will have a look
@jreback merged master
kk ping on green.
@jreback green, can be merged
thanks @phofl
CLN: avoid internals in DataFrame.sort_values
State: closed


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

Comments:
> Did you check that this didn't impact performance? Eg the verify default is different

I did not check perf, no.  Looking at it now I think I confused verify with allow_dups.

So it looks like this change adds an additional `((indexer == -1) | (indexer >= n)).any()` check, which is not huge but not nothing.

The upside is slightly less internals usage.  This is one of the more benign usages of _data, so id be OK with leaving this one
> which is not huge but not nothing.

Can you quantify "not nothing" for an example?

To be clear, if this has only a tiny impact, I am completly fine with using less internals in such functions (as that helps code clarity a lot). 
But sometimes there might have been good reasons internals were used directly, so let's at least check. It could also be a reason to expose some fastpaths (like in take here) in the public interface, if that would turn out ot be necessary.
Mothballing to clear the queue, will revisit.
CI troubleshoot azure
State: closed

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


Labels:
name: CI
description: Continuous Integration

Comments:
cc @TomAugspurger xfailing the affected test based on locale check fixed it, not an ideal long-term fix
so dateutil version changed?
Not sure what happened; Tom speculated in #33077 that an s3fs version bump might have done something.
k great
@Meeseeksdev backport to 1.0.x
TYP: require _update_inplace gets Frame/Series, never BlockManager
State: closed


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

Comments:
ok shouldn't you change the doc-string / annotation of _update_inplace (and maybe add an assertion) to guarantee this?
> and maybe add an assertion

Tricky since we could be dealing with a subclass.  I think its pretty clear in context.
are remaining comments deal-breakers?  nice follow-ups to de-duplicate some code after this
no my point on the _update_inplace method should only accept Series/Dataframe and not a BlockManager anymore 

i think that’s reasonable to add
> no my point on the _update_inplace method should only accept Series/Dataframe and not a BlockManager anymore

Right, thats what this PR does.  The docstring is updated to reflect that.  Just havent added an annotation because we dont have a ready-made "same type as self"
k fair enough

maybe an assertion would be good at some point
DOC: Added docstring for show_versions()
State: closed

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


Labels:
name: Docs

Comments:
@TomAugspurger, connected with issue #33044
You can ignore the CI failure.
Hello @MarianD! 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-03-30 14:23:01 UTC
@MarianD Can you please merge master?
> 
> 
@MomIsBestFriend 
> @MarianD Can you please merge master?

I merged my “docstring” branch into “master” in my fork and created pull request for that branch (master) — is it what you expected?


> > 
> 
> @MomIsBestFriend
> 
> > @MarianD Can you please merge master?
> 
> I merged my “docstring” branch into “master” in my fork and created pull request for that branch (master) — is it what you expected?

@MarianD sorry this is not what I expected, but it's perfectly fine.

I should have been more specific, what I meant was "please update your pull request"

https://dev.pandas.io/docs/development/contributing.html#updating-your-pull-request

This is necessary due to a failure we had in our CI (continues interrogation), and updating your PR (pull request) would solve this.
Thanks!
BUG: Add test to ensure, that bug will not occur again. #33058
State: closed

- [x] xref #33058
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Should I add a whatsnew entry? It's not really new, if it is backported to 1.0.1.

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

Comments:
Thanks @phofl for the PR. changing closes #33058 to xref until a decision re backport.

> Should I add a whatsnew entry?

not needed
marked as 1.1 but could backport if fixed on 1.0.x
@phofl can you merge upstream master
Hello @phofl! 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-02 19:12:29 UTC
@simonjayhawkins Done.
/azp run
<samp>
Azure Pipelines successfully started running 1 pipeline(s).<br>

</samp>
> So I'm not sure this is worth adding a test for - mutating the argument during a `.apply` seems fraught with peril and I'm a little hesitant to make guarantees about behavior
> 
> @jorisvandenbossche do you have a strong preference towards adding this?

this is fine, we currently support mutating inside an apply, though we have an issue to deprecate.
sounds good
Thanks @phofl 
Sorry @jreback merged while you were commented; i can do in a follow up
thanks @WillAyd
BUG: AttributeError when read_sas used with GCS
State: closed

With GCSFS (and when a binary file buffer is passed in), the output from
the file read is `bytes`, not a Unicode `str`. The `encode` call is unnecessary
in this case.

- [x] closes #33069 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: IO SAS
description: SAS: read_sas

Comments:
Not sure why "20.00% of diff hit (target 50.00%)", since I added a test and it's only one additional line of code. Maybe needs another test in text-mode?

Edit: None of the available XPT data files can be opened as a text file. It seems the `UnicodeEncodeError` passthrough may no longer be required at all with Python 3.
@bashtage if you'd have a look.
@tswast can you merge upstream master to resolve conflicts.
Linting
Thanks @tswast
BUG: Fix unwanted type casting while replacing values in a DataFrame
State: closed

- [x] closes #32988
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

As mentioned in #32988 [here](https://github.com/pandas-dev/pandas/issues/32988#issuecomment-604727057) I think, that I have found a way to fix this, but I don't know, if this results in a desired behavior. 

I would add tests after ensuring, that this does not break anything else. 

Any thoughts about this?

Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Regression
description: Functionality that used to work in a prior pandas version

Comments:
Thank, I'll follow this way in the future. 

I added a test.

When you use a value, which is actually replaced in the test, the pd.NA is converted to np.nan, if the original DataFrame had float64 as datatype. The example given in the issue did not replace a value from the original DataFrame.

Don't know, if this is a desired behavior.
i've labelled this as a regression, see https://github.com/pandas-dev/pandas/issues/32988#issuecomment-605519281 cc @jbrockmendel 
can you merge master and see if you can simplify
Thanks @phofl for looking into this. #32988 has been addressed via #34048.