BrandonFors/pandas-github-issues huggingface dataset:
CLN: dont catch on groupby.mean
State: closed

The new casting in cython_agg_blocks is specific to a single IntegerArray test case.  We could pretty reasonably move that into maybe_downcast_numeric.  For the moment id rather hold off since I expect other EA cases to show up here.

cc @WillAyd

Labels:
name: Groupby

Comments:
I think you commented this on my other PR but I have it as a goal to get rid of the block management altogether from groupby. This should be OK in the interim though
BUG: Allow all int types for merge (GH28870)
State: closed

- [x] closes #28870
- [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

Comments:
thanks @jjlkant 
a side issue, i suspect we don't have asv (performance tests for all of the int indexers) (and its possible since we don't have implementations for some of the non-int64 indexers they may be non-performant).

would you be up to submitting a PR for this?

https://github.com/pandas-dev/pandas/issues/28922
use requests when it is installed
State: closed

closes #16716
closes #28825
closes #28826
solves https://github.com/pandas-dev/pandas/pull/16910

- [ ] tests added / passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Continuation of https://github.com/pandas-dev/pandas/pull/21504 and https://github.com/pandas-dev/pandas/pull/17087

This PR is not ready but I'd appreciate any early comment/feedback.

Labels:
name: Enhancement
name: API Design
name: IO CSV
description: read_csv, to_csv
name: IO Excel
description: read_excel, to_excel
name: IO JSON
description: read_json, to_json, json_normalize
name: IO HTML
description: read_html, to_html, Styler.apply, Styler.applymap

Comments:
Haven't read through the attached issue but this is a little more complicated than I would have hoped for. Is there a reason why you think this is in scope of pandas? Pandas can operate for the most part on items that adhered to the IOBase definition. 

I don't think dealing with the `requests.models.Response` object is within scope
> Haven't read through the attached issue but this is a little more complicated than I would have hoped for. Is there a reason why you think this is in scope of pandas? Pandas can operate for the most part on items that adhered to the IOBase definition.
> 
> I don't think dealing with the `requests.models.Response` object is within scope

I'm assuming you are taking about the `session` object, right? If so what did you hope for and what do you recommend me to modify to improve this?

Some background. At NOAA we need the `session` object in order to read URLs that are under password protected servers. I guess we can try the `.netrc` approach but it would be nicer to do this without relying on local files.
IIRC pandas_datareader supports requests Sessions
> IIRC pandas_datareader supports requests Sessions

I know. That is what inspired the original poster of this PR.

I'm traveling back home today and I'll address the review comments during the weekend. Thanks!
I guess I'm not clear on what Sessions are doing here. Neither of the issues that this closes reference Session objects anywhere - are you sure this closes them?

Can you add test cases?
@ocefpaf still active? Can you address comments and check CI failures?
I'll probably have some time for this during the end of the year break. If someone wants to pick it up from here before that please feel free to do so.
@ocefpaf if you can merge master and see if you can get passing would be great.
Hi @ocefpaf  - sorry to chase you up, just wanted to ask whether you're still working on this :)
Seems that there's some test failures, but I can't check them as it says "build not found"
Closing as I think stale, but certainly ping if you'd like to continue
TST: Use fixtures instead of setup_method for index tests
State: closed

The common index tests use `setup_method` to create a `dict` of indexes to test against:

https://github.com/pandas-dev/pandas/blob/df2e0813e053cc5bc924b2292ea8918a6b27f0e2/pandas/tests/indexes/test_range.py#L25-L30

This `dict` of indexes is then iterated over within the tests:

https://github.com/pandas-dev/pandas/blob/df2e0813e053cc5bc924b2292ea8918a6b27f0e2/pandas/tests/indexes/common.py#L359-L363

The bulk of this PR involves converting `self.indices` into a parametrized fixture of indexes, and adjusting the tests to support this (largely just unindenting).  I had to do this conversion for all indexes at once since common test code for all index classes utilizes this pattern, so the diff is fairly large, but it should be relatively simple changes.

I also had to make some changes to references to specific indexes as well (e.g. `self.index`, `self.strIndex`, etc.) since the `setup_method` code also directly set each index in the `dict` as a class attribute.

Labels:
name: Refactor
description: Internal refactoring of code
name: Testing
description: pandas testing functions or related to the test suite
name: Index
description: Related to the Index class or subclasses

Comments:
lgtm. @simonjayhawkins ok here?
> lgtm. @simonjayhawkins ok here?

@jreback I think good to merge and tidy up as a folow-up.
thanks @simonjayhawkins  agreed

nice @jschendel
TST: Fix xfails for non-box maybe_promote on integer dtypes
State: closed

Orthogonal to other outstanding maybe_promote PRs.

This one required pretty significant changes to the code.  Using `np.min_scalar_type` and `np.can_cast` cleans this up a bit, but it is still pretty verbose.  AFAICT there is no clear way to make it shorter without significantly sacrificing clarity.

In a follow-up I think L410-459 can be refactored out to a helper function.  Waiting on that until I figure out the boxed=True cases, which are still troublesome.

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

Comments:
thanks. I suspect this is technically an api internally, but we likely don't expose this to the user. This may also have some perf implications. But let's refactor this beast and remove the array stuff before doing any of that.
Yah definitely time for a refactor/cleanup pass
Vectorized ISO Format
State: closed

- [ ] closes #28180
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Needs work but could certainly use some guidance from @jbrockmendel 

This uses the dt -> ISO string formatting that is deeply nested in the JSON code. It doesn't handle time zones properly (see #12997), doesn't match what you would get by default from `Timestamp.isoformat` (different precision) and doesn't support Timedeltas yet. When Timedeltas are supported this could ease some of the performance issues @cbertinato is seeing in #28595

In any case looking for guidance and thoughts on how to properly implement this, if this is even in the right direction

Here's a rough benchmark on performance:

```ipython


Labels:
name: Datetime
description: Datetime data dtype
name: Performance
description: Memory or execution speed performance

Comments:
> Is this the appropriate way to be registering the accessor method?

I think so.
There are a handful of np_datetime/np_datetime_strings funcs that we only use in the ujson code.  Would this replace that usage?  i.e. as a follow-up, could we get this func out of the C code and into cython?
This wouldn't replace it because this would only handle datetimelike objects when they appear in a DTA; ujson still handles these as one-off appearances in object arrays

Not sure about conversion to Cython. I think your idea to have this exposed in numpy would be ideal atm, so I'll check that route first and then maybe see if cythonizing makes sense thereafter
Might revisit this after #28912
CLN: catch less in pd.io
State: closed

@bashtage (only tangentially related to this PR) there is an `except Exception` in pandas.io.stata that I'd like to make more specific.  Any suggestions?

Labels:
name: Clean

Comments:
This is an intentional catch all that ensures the file gets cleanly closed.  It could legitimately encounter a wide range of errors include memory exceptions, io exceptions, windows exceptions and probably others. The only reason why Exception is caught is so it can re reraised. 

The `Exception` isn't actually needed since a bare `raise` will raise the origin exception, although if you remove it you will then have an except with no specified exception, which some don't like. 
Closing to clear the queue
TST: Fix 36 maybe_promote xfails wanting np.bytes_ instead of np.object_
State: closed

@jreback less trivial than some of the others.  These changes are based on my understanding that maybe_promote should _never_ be returning np.bytes_ dtype; those cases all become np.object_.  That is what maybe_promote currently does, and this updates the tests to expect that behavior.

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

Comments:
we should never have bytes as these are not a pandas supported dtype; anything that is not a standard (str, int, float, m8, M8 etc, or EA dtype) would just promote to object 

bytes are not special in that regard
> we should never have bytes as these are not a pandas supported dtype

agreed, that matches what this PR does.  The xfailed tests that this fixes do _not_ match what you said
thanks
ENH: Add optional argument keep_index to dataframe melt method (merged master onto old PR)
State: closed

This PR merges master onto @NiklasKeck's PR branch (#17440) to add an optional argument to keep_index to `pd.melt`. There is quite a bit of discussion between the following 2 PRs and issues:

Index gets lost when DataFrame melt method is used #17440
ENH: Add optional argument keep_index to dataframe melt method #17459
Melt enhance #17677

Please let me know if additional things need to be done to complete this PR

----

Setting keep_index to True will reuse the original DataFrame index +
names of melted columns as additional level. closes issue #17440

(cherry picked from commit 0c64bf0e5f145781c0f74fb93ffcd63a6d964bd9)

- [x] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


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

Comments:
Based on the decrease in code coverage, I assume tests will need to be added here:

https://github.com/pandas-dev/pandas/blob/df2e0813e053cc5bc924b2292ea8918a6b27f0e2/pandas/tests/reshape/test_melt.py#L9

I'm unfamiliar with how the tests are actually structured, so if someone could provide some direction, I could put together a test for review.
Yep tests are pretty much the most important part of any PR. If you can add someone can take a look
@smsaladi this needs tests for the new behavior
Closing as I think this is stale, but @smsaladi feel free to ping if you want to pick back up and address comments
DOC: Fix missing periods and non capitalized summary beginnings (#27977)
State: closed

- [ ] closes #27977 
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Fixed for the following functions
`pandas.io.formats.style.Styler.set_caption
pandas.plotting.table
pandas.plotting.register_matplotlib_converters
pandas.plotting.deregister_matplotlib_converters
pandas.arrays.IntervalArray.mid
pandas.Timedelta.ceil
pandas.Timedelta.floor
pandas.Period
pandas.Period.asfreq
pandas.Period.to_timestamp`


Labels:
name: Docs

Comments:
@WillAyd Cool, thanks for the quick review! And... *ping*, CI checks have passed.
Thanks, @DanBenHa !
CLN: assorted cleanups, mostly post-black fixups
State: closed

Mostly fixing extra " " introduced by black (will take a look at the issue tracker there to see if that can be fixed once and for all).

Also change `except Foo as e:` to `except Foo as err` and remove one `except Exception` in a docs file.  Clarified a whatsnew note that @jorisvandenbossche asked for a while back.

Labels:
name: Clean

Comments:
Hello @jbrockmendel! 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 2019-10-10 13:43:23 UTC
Opened an issue with black: https://github.com/psf/black/issues/1051
Thanks!
remove doc note about apply applying a function to the first element …
State: closed

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


Labels:
name: Docs

Comments:
Great thanks @tlynch1315 !
[MINOR] Fix formatting string in _ODFReader.get_sheet_by_name
State: closed

This PR fixes minor problem with `_ODFReader.get_sheet_by_name` method.  

If `name` is not found, it attempts to raise a `ValueError` with formatted string that expects keyword argument `name`, however `names` is passed as a positional argument. In effect it will throw unexpected `KeyError`.

Steps to reproduce the problem:

```python
from odf.opendocument import OpenDocumentSpreadsheet
import tempfile

path = tempfile.mktemp()
OpenDocumentSpreadsheet().save(path)
pd.read_excel(path, engine="odf", sheet_name="foo")

# Traceback (most recent call last):
...
# KeyError: 'name'

```

Expected behavior

```python
# Traceback (most recent call last):
# ...
# ValueError: sheet foo not found

```

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:

Comments:
Makes sense.  Can you add a test for this
This is a dupe of #27677 - @ilogue is that still active?
> This is a dupe of #27677 - @ilogue is that still active?

My bad, sorry.
No worries! Let’s see if still active if not can certainly reopen this
@WillAyd yes gtg from my end, waiting for new reviews
Clean up Abstract and Naming Definitions for GroupBy
State: closed

Summary of changes:

  - Made `_iterate_slices` abstract in base class, moved pre-existing definition from Base -> Series where it was actually being called (DataFrame already overrides)
  - Made `_wrap_transformed_output` Abstract in base class, as both Series and DataFrame override this
  - Redefined `_wrap_applied_output`; this was Abstract in base class before but its definition was not in sync with how the subclasses actually defined
  - Renamed `_wrap_output` to `_wrap_series_output` as it is only valid for SeriesGroupBy
  - Renamed `_wrap_generic_output` to `_wrap_frame_output` as it is only valid for DataFrameGroupBy
  - Renamed `_aggregate_generic` to `_aggregate_frame` as it is only valid for DataFrameGroupBy

More to come in separate PRs


Labels:
name: Refactor
description: Internal refactoring of code
name: Groupby

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 2019-10-08 16:15:58 UTC
makes sense.  LGTM pending green
thanks @WillAyd 

doc-strings would be very helpful on some of these as well :->
Agreed - docstrings and annotations should come in one of these
DOC: Fixed PR08 and PR09 docstring errors in pandas.Series
State: closed

Fixed all PR08 and PR09 issues in pandas.Series.

- [x] closes [#28738](https://github.com/pandas-dev/pandas/issues/28738)
- [x] passes `black pandas`


Labels:
name: Docs

Comments:
Thanks a lot for the fixes @jackbicknell14
TST: Improve compatibility with pypy error messages
State: closed

This fixes a bunch of spurious errors when running the test suite on pypy3.

- [ ] closes #xxxx
- [ ] 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: Error Reporting
description: Incorrect or improved errors from pandas

Comments:
Hmm I'm surprised pypy would impact these. Can you post pd.show_versions()?
```
INSTALLED VERSIONS
------------------
commit           : a9085e80b0a7dd07f398f2c0b87f68b5c680402e
python           : 3.6.9.final.0
python-bits      : 64
OS               : Linux
OS-release       : 4.4.0-160-generic
machine          : x86_64
processor        : x86_64
byteorder        : little
LC_ALL           : None
LANG             : en_GB.UTF-8
LOCALE           : fr_FR.UTF-8

pandas           : 0.26.0.dev0+502.ga9085e8
numpy            : 1.18.0.dev0+68bd6e3
pytz             : 2019.2
dateutil         : 2.8.0
pip              : 19.2.3
setuptools       : 41.2.0
Cython           : 0.29.13
pytest           : 5.1.3
hypothesis       : 4.36.2
sphinx           : None
blosc            : None
feather          : None
xlsxwriter       : None
lxml.etree       : None
html5lib         : None
pymysql          : None
psycopg2         : None
jinja2           : None
IPython          : None
pandas_datareader: None
bs4              : None
bottleneck       : None
fastparquet      : None
gcsfs            : None
lxml.etree       : None
matplotlib       : None
numexpr          : None
odfpy            : None
openpyxl         : None
pandas_gbq       : None
pyarrow          : None
pytables         : None
s3fs             : None
scipy            : 1.3.1
sqlalchemy       : None
tables           : None
xarray           : None
xlrd             : None
xlwt             : None
xlsxwriter       : None
```
`pd.show_versions()` is missing an important bit of information though, i.e.
```
Python 3.6.9 (16472df37081, Sep 27 2019, 01:00:13)
[PyPy 7.3.0-alpha0 with GCC 6.2.0 20160901] on linux
```
I suspect this is probably due to numpy versioning instead of pypy but in any case I think OK. @simonjayhawkins care to look?
LGTM.

Should we consider adding a CI build for pypy?
There are lots of failures ATM, I want to fix more of them before suggesting adding it to CI.
> There are lots of failures ATM

@rlamy yep, I'm seeing 750 failures and 35 errors.  Some of those are mismatching exception messages like this PR, but I'm also seeing a lot of OverflowErrors.  Are you involved with pypy?  i.e. are you able/willing to help troubleshoot these?
Not sure where we strayed from #9532 but @rlamy if something you are interested in supporting I would suggest opening a dedicated issue and ideally coming up with bullet points on things that need to be tackled
DOC: fix code-block in the reshaping docs
State: closed

- [x] closes #28819



Labels:
name: Docs

Comments:
When I review file It looks fixed but checks failing :cry: 
https://github.com/pandas-dev/pandas/pull/28838/files?short_path=47c625e#diff-47c625e8dc3265ed493265a79b1e823c
I think we should change ".. code-block::"  to ".. code:: " so its about linting problem.

This is a [useful link](https://gitlab.com/pycqa/flake8/commit/9553c8d8ccf54221153f498e28297d9cc74ca12d)
Finally I found the solution.  
`E999 SyntaxError: invalid syntax causes flake8 to skip AST tests. Keep mandatory E999 issues in blocks with :flake8-group: Ignore to preserve full testing for the rest of the blocks.`
[link is here](https://pypi.org/project/flake8-rst/)
if I add a whitespace after .. code-block:: `flake8-rst` works good but your script `! grep -n '--exclude=*.'{svg,c,cpp,html,js} --exclude-dir=env -RI "\s$" * | awk -F ":" '{print "##vso[task.logissue type=error;sourcepath=" $1 ";linenumber=" $2 ";] Tailing whitespaces found: " $3}'` gives error.

So I will change code-block to .. code::

If I convert it to `.. code::` it won't give `E999` flake8 error and `tailing whitespaces` error.

I test it manually from my local computer. I hope it will work and I think this is a bug. 
Thanks! Did you render the html? do you mind copying a screenshot to see how it looks?
> Thanks! Did you render the html? do you mind copying a screenshot to see how it looks?

Yes I rendered it on my local machine. 

[image link](https://www.imgpaste.net/image/rGT06)

( *imgur was banned in Turkey. So I uploaded the screenshot to this site.* )
@oktaysabak Thanks for the screenshot! That's looking good, and CI is passing now ;) So merging, thanks again for your contribution!
This is my first merge in a big project. Thank you !
Remove NDFrameGroupBy Class
State: closed

Was going to do a larger refactor but this diff looks more confusing than it actually is, so figured I'd stop here for readability. All I've done in this PR is:

  - Remove NDFrameGroupBy, consolidating it's methods into DataFrameGroupBy
  - Merge NDFrameGroupBy.aggregate into DataFrameGroupBy.aggregate (the latter previously called the former)

After this I am looking to find a more logical home for the functions, as the current hierarchy isn't super clear. For example, `_iterate_slices()` should probably be abstract in the base `GroupBy` class, but right now the Series implementation is in the superclass while DataFrameGroupBy overrides that for itself. Series.pct_change is practically the same as GroupBy.pct_change, so is probably unnecessary. Will be a few more follow ups mixed in

Labels:
name: Refactor
description: Internal refactoring of code
name: Groupby

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 2019-10-08 06:05:46 UTC
Yep...like I said I think the way git is calculating the diff on this makes it seem a lot bigger than it is. Mostly just a cut and paste of the entire class def

Sent from my iPhone

> On Oct 8, 2019, at 3:18 AM, Jeff Reback <notifications@github.com> wrote:
> 
> ﻿
> @jreback requested changes on this pull request.
> 
> you know how we love large PRs!
> 
> but will look
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.

> Mostly just a cut and paste of the entire class def

where is this NOT a cut-paste? can you comment on the code
It might also be helpful to look at the commits besides the blackify one to see the only things that aren’t direct copy paste. 

The initial commit is the entire cut and paste, just difficult to see that by default with git
thanks @WillAyd copy-pasting is ok here!

yeah this makes sense..