BrandonFors/pandas-github-issues huggingface dataset:
TYP: Annotate groupby/ops.py
State: closed

Adding a few type annotations to this file. I think the `values` arguments here should be `ndarrays` but am not positive.

Labels:
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
Hello @dsaxton! 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-26 21:53:19 UTC
merge master
LGTM.  @simonjayhawkins are we at a point where mypy would tell us if anything here were incorrect?
> LGTM. @simonjayhawkins are we at a point where mypy would tell us if anything here were incorrect?

using the word 'incorrect' I think would only apply if we have 100% static typing. I don't think we  have an appetite for that (achieving consensus in the pandas team). By 100% static typing, I mean 0% dynamic types. That means nothing resolves to Any. That includes all imports and cython code. It also includes code patterns that create classes, methods and attributes etc dynamically. We have alot of dynamic code.

Mypy tells us if the types added here are 'consistent' with the other types already added. The more types added and the more refined the types the better the 'correctness'. Again, there does not seem to be an appetite for some patterns that refine types or workflows that expedite the addition of types.

IMO with typing, 'Job done is better than perfect'. Simply put, the more types added overall, the more confidence we can get for refactoring, enhancement PRs etc.

so PRs that add types, keep mypy green and are not obviously wrong get a thumbs up from me. 

partial typed function signatures are better than none. unparameterized generics are more refined than Any. If we did want to enforce this we would use `--disallow-incomplete-defs` and `--disallow-any-generics`. Mypy can do this checking so that reviewers don't need to.
 
can you merge master, ping on green.
> can you merge master, ping on green.

@jreback Merged and green
CLN: Remove unused is_datetimelike arg
State: closed


Labels:
name: Clean

Comments:
are there any situations where we should be passing is_datetimelike in to the cython functions?
> are there any situations where we should be passing is_datetimelike in to the cython functions?

I'm not sure to be honest. Should the functions be able to infer that from `result` if needed?
> I'm not sure to be honest. Should the functions be able to infer that from result if needed?

I don't think so; any of the datetimelike dtypes will be cast to i8 before being passed to the cython funcs.

Even if we don't have a good answer for that question, this PR certainly doesnt make matters any worse, and is a nice cleanup.  LGTM.
thanks @dsaxton 
yeah this looks like some needed cleanup after a previous couple of refactortings.
DOC: Modify validate_rst_title_capitalization.py script
State: closed

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


Labels:
name: Docs
name: CI
description: Continuous Integration

Comments:
Thanks for this @cleconte987 

Can you please explain what you are proposing here, and why? It's difficult to review without referencing an issue, or providing a description.

My understanding from your other PR is that we wanted to add a lot of keywords to the list of accepted keywords. I guess this is related, but not sure why you think this is a better approach.
Sorry I didn't put any information, I didn't opened or referenced an issue.

Yes it is related to the other PR. What I wanted to do was to not remove uppercases from words which are proper nouns. From what I understand, all words with two or potentially more uppercase letters are proper nouns and should be kept as is. So the snippet's aim is to separate words which have 2 or more uppercase letters from other words, and to only lower the second ones.
So, if I understand correctly, with this change, if I write `PANDAS`, `FooBar` or whatever with more than two upper cases, it will be considered valid, right?

I'm personally -1 on this, I would prefer to be explicit on what we accept and we don't. I think the list of exceptions is big, but still ok.
> So, if I understand correctly, with this change, if I write `PANDAS`, `FooBar` or whatever with more than two upper cases, it will be considered valid, right?
> 
> I'm personally -1 on this, I would prefer to be explicit on what we accept and we don't. I think the list of exceptions is big, but still ok.

Yes exactly, words like `PANDAS` and `FooBar` will be considered valid. But they still be changed in the end by the correct spelling from CAP_EXCEPTIONS_DICT if they belong to it.

Ok I will add elements to CAPITALIZATION_EXCEPTIONS then
Can you add them in the other PR for now, and leave this open?

@WillAyd what do you think about this? Feels like making things too complicated, but may be it's a good idea. 
Ok sure.

I will add them meanwhile right
Wouah I accidentally deleted my remote branch on GitHub and closed the PR :/
Sorry for that
I will reopen one
BUG: Fix min_count issue for groupby.sum
State: closed

- [x] closes #32861
- [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: NA - MaskedArrays
description: Related to pd.NA and nullable extension arrays

Comments:
@dsaxton what's the status here?
> @dsaxton what's the status here?

It's working / green, but I think @jreback may want it refactored (I haven't had much success in deleting what looks like redundant code without causing tests to fail)
The 'regression' for Int64 could be considered a bug since Int64 is considered experimental. 

I've not looked in detail yet, but the 'regression' was caused by #31359 see https://github.com/pandas-dev/pandas/issues/32861#issuecomment-605453895 so would to check that the other EAs which aren't experimental aren't affected by this issue.


> but I think @jreback may want it refactored

**if** this issue **does** affect other EAs, then this PR is a candidate for backport, so would rather keep the changes here minimal.
> **if** this issue **does** affect other EAs, then this PR is a candidate for backport, so would rather keep the changes here minimal.

It doesn't affect others that I'm aware of (there's a similar but as far as I can tell unrelated problem with boolean data type which I created a separate issue for)
@jreback @simonjayhawkins Is is possible we can merge this as is? I have a fix for a separate bug that depends on this.
> @jreback @simonjayhawkins Is is possible we can merge this as is? I have a fix for a separate bug that depends on this.

sure is this rebase on master?
> sure is this rebase on master?

I think Simon merged this morning
> > **if** this issue **does** affect other EAs, then this PR is a candidate for backport, so would rather keep the changes here minimal.
> 
> It doesn't affect others that I'm aware of (there's a similar but as far as I can tell unrelated problem with boolean data type which I created a separate issue for)

there is a problem also with timedelta, see https://github.com/pandas-dev/pandas/issues/34051#issuecomment-626688541, but this also occurs on 0.25.3

so won't backport for now.
ENH: Add ods writer
State: closed

- [x] closes #27222
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry
- [x] add support for startrow and startcol parameters

Labels:
name: IO Excel
description: read_excel, to_excel

Comments:
@WillAyd I am not sure where "the lot of copy paste" comes from. It is true that I started off from `_xlwt.py`, but there are just 20 unmodified lines of the 321. Most of which are the standard methods required for each engine.
Hello @roberthdevries! 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 12:34:37 UTC
Can you merge master - thanks!
Rebased to master
@roberthdevries can you merge master and will look again.
can you rebase and update
Great thanks @roberthdevries 
You're welcome
WIP: Draft strawman implementation of draft strawman data frame "__dataframe__" interchange protocol for discussion
State: closed

xref https://github.com/wesm/dataframe-protocol/pull/1

Labels:
name: API Design
name: Needs Discussion
description: Requires discussion from core team before further action
name: DataFrame
description: DataFrame data structure

Comments:
Trying to plow through the PR backlog; is this actionable?
closing to clear queue.
TST: Avoid bare pytest.raises in mult files
State: closed

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

Comments:
Hello @Vlek! 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-22 18:56:22 UTC
I haven't the slightest clue why pandas-dev.pandas is taking so long while the others were successful. I would appreciate help in figuring out what's wrong.
thanks @Vlek
Fix to _get_nearest_indexer for pydata/xarray#3751
State: closed

@jbrockmendel thanks for encouraging me to submit a PR.  I'm totally open to any suggested changes; this is just a sketch of something that would work for our needs and hopefully not break anything in pandas.

To those not familiar, some changes introduced in https://github.com/pandas-dev/pandas/pull/31511 broke some functionality of an Index subclass we define in xarray.  More details/discussion can be found in https://github.com/pydata/xarray/issues/3751 and https://github.com/pydata/xarray/pull/3764, but the gist of the issue is that for better or for worse, we have been relying on left and right distances in `_get_nearest_indexer` being determined using NumPy arrays rather than Index objects (as they were prior to https://github.com/pandas-dev/pandas/pull/31511).  

One way to fix this is to override `_get_nearest_indexer` within our Index subclass, which we've temporarily done in https://github.com/pydata/xarray/pull/3764, but this potentially fragile.  It would be great if we could find a solution here.

- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Dependencies
description: Required and optional dependencies

Comments:
The test I added fails in CI, because of another issue, which I proposed a downstream fix for in https://github.com/pydata/xarray/pull/3874#issuecomment-602046357.
Looks like `CFTImeIndex.__new__` needs to instantiate `_cache`; for now i guess the test will have to monkeypatch
It occurred to me we could write a more direct test that circumvents the `_cache` issue for now; that way we won't need to temporarily monkeypatch.
Looks like test failures are happening because we are calling _filter_indexer_tolerance inconsistently (in master, unrelated to this PR).

To fix this, I think we need to go back to your way of doing it: pass target._values to `_filter_indexer_tolerance`, and we'll have to update the annotation
Cool, that seemed to do it; looks like things are all green now.  Let me know if you have any further comments.
I think this may be ready now.  Does this require a what's new entry?  If so, I notice this was tagged for a 1.0.4 release.  Should I create a new what's new file for that?
Great, thanks @jreback.  I added a what's new entry.  Let me know if you'd like me to tweak it at all.  I appreciate your and @jbrockmendel's help with this.
lgtm ping on green.
Many thanks @jreback; looks like things are green.
thanks @spencerkclark very nice! keep em coming!
EHN: to_{html, string} col_space col specific
State: closed

- [ ] closes #28917
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

In to_html, to_latex and to_string, ```col_space``` arg now accepts a list or dict where we can mention the column name to change only a specific column's width.
Haven't add test for to_latex and to_string since they too don't have test for the old col_space and I cannot think of anything less cumbersome than to count added whitespace.

Labels:
name: Enhancement
name: IO HTML
description: read_html, to_html, Styler.apply, Styler.applymap

Comments:
@quangngd you still interested in working on this?
> @quangngd you still interested in working on this?

Yes, Im still waiting for feedbacks
Hello @quangngd! 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-18 06:38:51 UTC
Tests added for to_string.
Decided to drop to_latex due to lack of latex experience.
FYI, the current implementation for col_space in to_latex doesn't work either since it just simply add spaces between strings (like in to_string).
@quangngd - thanks for the PR mind fixing up the test failures? 
@alimcmaster1 done
cc @WillAyd @simonjayhawkins for a look
Great PR - thanks a lot @quangngd !
DOC: Fix EX02 in pandas.Series.factorize
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Related to #27977. 

output of `python scripts/validate_docstrings.py pandas.Series.factorize`:
```
################################################################################
################################## Validation ##################################
################################################################################
```



Labels:
name: Docs

Comments:
> 
> 
> lgtm, and CI is green. Is this working for you locally @farhanreynaldo ?

Yes, it's working now.
Thanks @farhanreynaldo !
Timedeltas: Understand µs
State: closed

- [ ] closes #xxxx
- [X] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry


These are emitted by golangs `time.Duration` printing: https://github.com/golang/go/blob/36b815edd6cd23d5aabdb488c24db2033bbdeea2/src/time/time.go#L669

Labels:
name: Bug
name: Timedelta
description: Timedelta data type

Comments:
> pls add a test & a what’s new note (other enhancements)

Done

> is this fixing a specific issue?

Yes, this is fixing the "Pandas can not parse golang duration strings if they contain microseconds" issue.
what i mean is do we already have an open issue for what this is fixing?
> what i mean is do we already have an open issue for what this is fixing?

No, want me to open one and add a closes directive to the PR?
@jreback The `CI / Checks (pull_request)` failed with a `HTTPError: HTTP Error 503: Backend fetch failed`. That's not from the PRs changes I guess? Can you restart it or do I need to force push?
no don’t open an issue
@alvaroaleman Can you please rebase? the CI is failing due to an issue we recently resolved, merging master should solve this.
@MomIsBestFriend done, all checks are green now :)
thanks @alvaroaleman
TST: test that the .copy method on indexes copy the cache
State: closed

In #32883 I made a PR to fix an issue that ``MultiIndex.copy`` didn't copy the ``_cache``.

This adds a test for this for all index types (there's no futher issue, but it should still be tested for).

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

Comments:
thanks @topper-123
CLN: Avoiding casting
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

---

Follow up to https://github.com/pandas-dev/pandas/pull/32794#discussion_r394045101


Labels:
name: Clean

Comments:
Looks good. Could reduce the number of comments but not a blocker for me

@jbrockmendel were there any reasons to use shape instead of len with memory views? I think I remember you mentioning something about that. 
> were there any reasons to use shape instead of len with memory views? I think I remember you mentioning something about that.

Yah, for nearly the exact same reason its being done here: the `.shape` lookup happens in c-space while the `__len__` call goes into pyspace.  IIRC thats unintentional and expected to be changed in cython 0.30

LGTM; agreed that the comments can be less verbose, just one-liners `TODO(cython0.30): use len instead of shape[0]`
> LGTM; agreed that the comments can be less verbose, just one-liners `TODO(cython0.30): use len instead of shape[0]`

@MomIsBestFriend can you do this?
thanks @MomIsBestFriend 

should make an issue to change this once cython 0.30 is out
CLN: Fix common spelling mistakes
State: closed

I applied [this awesome automatic spell fixer](https://github.com/vlajos/misspell-fixer) to our codebase. I needed to do a manual review as it was attempting to fix some names, but the false-positive rate was pretty low!

They also provide a GitHub action for automatic checks on PRs. Might be helpful, especially for the `docs`.

If this is merged, I can create a follow-up issue for a discussion on the CI hook.


Labels:
name: Docs
name: Clean

Comments:
looks fine ping on green.
/azp run
<samp>
Commenter does not have sufficient privileges for PR 32895 in repo pandas-dev/pandas<br>

</samp>
@jreback CI is green
Thanks @SaturnFromTitan
[BUG] Sum of grouped bool has inconsistent dtype
State: closed

- [x] closes #7001
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Would appreciate any feedback on this attempt.

The strategy is to modify the dtype after the aggregation is computed in certain cases when casting. In order for this to work, the cast functions need to be made aware of how the data was aggregated. I've added an optional "how" argument to maybe_downcast_numeric and _try_cast. Because this dtype change is needed in two places, I've added the function groupby_result_dtype to dtypes/common.py to handle the logic.

I wasn't sure where the mapping information needed by groupby_result_dtype should be stored. Currently it is in the function itself, but maybe there is a better place for it.

If this is a good approach, it could potentially be expanded for other aggregations and datatypes. One thought is that perhaps groupby(-).mean() should always return a float for numeric types.

Labels:
name: Groupby
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
@rhshadrach if you can update
@jreback Failure is unrelated, ready for another review:

``
##[error]./pandas/tests/indexes/timedeltas/test_constructors.py:173:String has a space at the beginning instead of the end of the previous string.
``
@jreback ping
@jreback ping
@jreback ping
very nice @rhshadrach keep em coming!

Thanks for the feedback @jbrockmendel. I'll clean these type hitns up in a subsequent PR.
> I'll clean these type hitns up in a subsequent PR.

FYI mypy error:  `pandas\core\dtypes\cast.py:333:18: error: "type" has no attribute "_from_sequence"`

so maybe worth adding type hints to `maybe_cast_to_extension_array` and add a subclass check to the instance check
Added `const` where avaible
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: Performance
description: Memory or execution speed performance

Comments:
any perf change?

cc @WillAyd @jbrockmendel 
LGTM
> any perf change?
> 
> cc @WillAyd @jbrockmendel

Unsure which function to test, which one you would like to see?
~@jreback Actually I can't manage to run ```asv```, I hoped you can give me a specific function/method name to ```%timeit``` on~ Posted below (not sure if I did it right)
```
[ 75.00%] ··· tslibs.timestamp.TimestampOps.time_tz_convert                                                 ok
[ 75.00%] ··· ============ =============
                   tz                   
              ------------ -------------
                  None        296±4ns   
               US/Eastern    17.8±0.2μs 
                 <UTC>      5.11±0.04μs 
                tzutc()      5.17±0.2μs 
              ============ =============

[ 75.00%] · For pandas commit 4195bfbe <master> (round 2/2):
[ 75.00%] ·· Building for conda-py3.6-Cython-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt...
[ 75.00%] ·· Benchmarking conda-py3.6-Cython-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[100.00%] ··· tslibs.timestamp.TimestampOps.time_tz_convert                                                 ok
[100.00%] ··· ============ =============
                   tz                   
              ------------ -------------
                  None        309±3ns   
               US/Eastern    17.6±0.4μs 
                 <UTC>      4.90±0.03μs 
                tzutc()      5.11±0.1μs 
              ============ =============


BENCHMARKS NOT SIGNIFICANTLY CHANGED.
```
Thanks @MomIsBestFriend
REF: simplify should_extension_dispatch, remove dispatch_to_extension_op
State: closed

dispatch_to_extension_op was needed back before we got rid of integer-addition for DTA/TDA/Timestamp/Timedelta.  Now it reduces to a one-liner, which this inlines.

This also consolidates all of the casting/checks up-front, which in turn avoids a runtime import and simplifies the check for should_extension_dispatch.

Labels:
name: Refactor
description: Internal refactoring of code
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
thanks