BrandonFors/pandas-github-issues huggingface dataset:
TST/CLN: replace %s formatting syntax with .format in tests
State: closed

xref #16130

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

Comments:
thanks @simonjayhawkins 

are we in a position to code_check and fail for the old format?
> are we in a position to code_check and fail for the old format?

i've only done the test directory here. we could start with a code check covering just that?
> > are we in a position to code_check and fail for the old format?
> 
> i've only done the test directory here. we could start with a code check covering just that?

sure, though I think *most* of the codebase is good actually, but happy to start having a check and can expand it later.
BUG: Fix insertion of wrong-dtypes NaT into Series[m8ns]
State: closed

Broken off from #27311 to troubleshoot build-specific failures.  Also re-wrote test to parametrize and be more succinct

Labels:
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

Comments:
> not sure I agree with the point of this PR. the np.* based 'NaT' should never exist in pandas at all, ever, we simply need to cast them appropriately

xref #24983.  What you are suggesting is 100% impossible to do correctly.

> I get that you want to turn an M8 array when presented with an m8('NaT') into object, but I do't think we currently do

Well you're right that it is not what we currently do, but that is a bug.  It is unambiguously the correct behavior.

> However I don't view this distinct as very import and it adds a lot of complexity

It's your prerogative (a word whose spelling never ceases to amaze me) to consider it a low-priority bug.  On the flip side, I see complexity stemming from the fact that we have to guess what dtype NaT is behaving as in any given situation, and the impossibility of always guessing correctly.

> We now treat NaT, np.nan, None and np.*('nat') synonymously as missing values when presented to say a M8 (or a m8); I don't see why we don't simply say, hey I have a missig value, great use the value for this dtype.

Consider: if a user passes `pd.NaT`, that does basically your "hey I have a missing value" thing.  But if a user _specifically_ passes `np.datetime64("NaT")`, they did that on purpose.
changed test class to just a function
rebased following can_hold_element fix
gentle ping; this one is a bugfix, admittedly a small one
sure
thanks @jbrockmendel
BUG: fix inserting tz-aware datetime to Series, closes #12862
State: closed

- [x] closes #12862
- [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: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Timezones
description: Timezone data dtype

Comments:
comments addressed i think
ok thanks; this looks better.
BUG: Consistent division by zero behavior for Index/Series
State: closed

- [x] closes #13843

We have two very-similar functions for masking division by zero for Index vs Series.  The Index version is the more correct of the two, so I'm trying to make the Series one behave more like the Index one, eventually just using the Index one for both.

The hard part here is that IntegerArray, SparseArray, and SparseSeries have their own arithmetic implementations, and we test that they behave the same as Series.  Several attempts to patch these implementations have failed (and motivated #27302, which this is rebased on top of), so for now this PR patches the tests instead of the implementations.

I think the IntegerArray implementation should be relatively easy to patch.  SparseArray I'll need some help (cc @TomAugspurger) with `__setitem__` and/or `putmask`.


Labels:
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
k looks fine (except for the comments which can be addressed as a followup), but we need a note for the changes in Series yes? (e.g. they are very edge casey but need a mention)
will add note.  do we want to try to fix the IntegerArray and SparseArray behavior here too?  IntegerArray I think I can figure out but SparseArray I've beat my head against and got nowhere.
> will add note. do we want to try to fix the IntegerArray and SparseArray behavior here too? IntegerArray I think I can figure out but SparseArray I've beat my head against and got nowhere.

no let's do as followups.
lgtm ping on green (note that I just merged a patch to make the numpy dev branch pass) so you may need to rebase again.
ping
TYPING: Partial typing of Categorical
State: closed

Adds some typing to ``Categorical``.

This PR does the simpler typing of ``Categorical``. In addition there are some more thorny typing that I've stayed away from for now to see if others can implement the basic types (list-likes etc.).

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

Comments:
looks good, small comment, merge master and ping on green.
Can we address my comment on `OrderedType` before merging? I don't think the definition in pandas._typing of that should add `object` because one method in the code base uses that as a temporary workaround
can you rebase
Rebased.
lgtm. over to you @WillAyd 
Great thanks a lot @topper-123
BUG: Preserve CategoricalDtype._ordered_from_sentinel with pickle
State: closed

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


Labels:
name: Bug
name: Categorical
description: Categorical Data Type

Comments:
thanks @jschendel very nice
CLN: Import abc from collections.abc in prep for py3.8
State: closed

 #23750, and various issues on supporting upcoming python 3.8.

Azure (multiple):
```
[warning] Using or importing the ABCs from 'collections' instead 
of from 'collections.abc' is deprecated, and in 3.8 it will stop working
```

From https://docs.python.org/3/library/collections.html?highlight=collections#module-collections:
```
Changed in version 3.3: Moved Collections Abstract Base Classes to the 
collections.abc module. For backwards compatibility,  they continue to be 
visible in this module through Python 3.7.  
Subsequently, they will be removed entirely.
```


Labels:
name: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
You're right, I saw the test warning looked it up and completely misread the release notes. 
It's `from collections import ABCfoo` which will break with 3.8, and I can't find any. Sorry for the noise.


No problem, the warnings are not always very clear in where they are coming from ..
BUG: Fix inserting of wrong-dtyped NaT, closes #27297
State: closed

- [x] closes #27297
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Needs tests for PeriodDType case.  The tests are probably not in the ideal place, how to move/parametrize them depends on what axis we want to sort them along (i.e. all Series indexing tests together or all timedelta-insertion tests together)

Labels:
name: Datetime
description: Datetime data dtype
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

Comments:
> this patch is very awkward. This needs work to make it more fluid.

Might be because it addresses three separate-but-related bugs.  Looking at it commit-by-commit might make more sense.  I'm happy to make separate PRs if that's easier
Rebased following #27323.  This does for datetime64 Series what that did for timedelta64 Series
no whatsnew, so doesnt need to be moved
> Why for example would the ObjectBlock construct not simply reshape (if needed); it already knows the dim & the values shape?

Because the Block constructor calls self._validate_ndim to check that it got the expected shape.  reshaping is done earlier.
> > Why for example would the ObjectBlock construct not simply reshape (if needed); it already knows the dim & the values shape?
> 
> Because the Block constructor calls self._validate_ndim to check that it got the expected shape. reshaping is done earlier.

is this not exactly what ``_block_shape`` does? 
> is this not exactly what _block_shape does?

It is, but block_shape gets called before the constructor.

But apparently it isnt necessary.  Just ran the tests with this reshape disabled and its fine now.  Will remove.
removed+green
whatsnew added + green
thanks @jbrockmendel 
It looks like this is causing a bunch of CI failures.  cc @jreback @TomAugspurger
STYLE: fix line length check of flake8
State: closed

I think I made a small mistake in https://github.com/pandas-dev/pandas/pull/27076 in adding the E501 code (line length) to the ignore list, as setting the flake8 max line length to 88 should have been sufficient.

Black does not check or correct line length in comments or multiline strings (eg docstrings), so this still need to be catched by flake8

Labels:
name: Code Style
description: Code style, linting, code_checks

Comments:
But, this gives some interesting cases, it seems not always easy to reconcile both ... 
thanks @jorisvandenbossche
CLN: Split test_window.py
State: closed

xref #19228, #26807

- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

`test_window.py` was getting fairly unwieldily. There a couple test classes that use a non-pytest-idomatic `Base` class, so those test classes are kept together for now. I split out the rest of the test classes into separate files as they made sense.

I think the classes in `test_window.py` need further untangling before a `conftest.py` file can be made for the new directory.

Open to additional quick win optimizations.

cc @jreback 

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Clean
name: Window
description: rolling, ewma, expanding

Comments:
@mroeschke @WillAyd let's do the straight split first, modifications later.

ping on green.
I think we merged something, can you merge master and ping on green.
All green @jreback 
thanks @mroeschke 
it looks like this broke the ci.
@simonjayhawkins can u put a link up 
https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=14275
Strange that my PRs based on earlier commit also fails. Does azure merge the PR into master before tesing? 
> Does azure merge the PR into master before tesing?

it looks like it. good to know.
PERF: Suppress ix warnings benchmarks
State: closed

- [x] closes #27217
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


~~TODO: add tests for suppressed warnings.~~


Labels:
name: Performance
description: Memory or execution speed performance
name: Deprecate
description: Functionality to remove in pandas

Comments:
@jreback for review. Thank you!
@leeyspaul I think you are adding the filterwarnings in the wrong place. We don't want to suppress those warnings in general (that's why we put them there, so users see them), but only in the benchmarks. So it is inside the benchmarking code that we would need to add filters.
@jorisvandenbossche Ahhh right, thank you for that. I believe I've suppressed the appropriate warnings in the right places this time around! Please take a look when able. :)
thanks @leeyspaul
BUG: appending a Timedelta to Series incorrectly casts to integer
State: closed

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


Labels:
name: Timedelta
description: Timedelta data type

Comments:
thanks, pls add the comments to list of followups
check early for non-scalar default_fill_value
State: closed

Broken off of an in-progress division-by-zero branch

Labels:
name: Error Reporting
description: Incorrect or improved errors from pandas
name: Sparse
description: Sparse Data Type

Comments:
is there an issue for this? I guess currently this raises but its an incorrect description of the error?
> is there an issue for this? I guess currently this raises but its an incorrect description of the error?

ATM this raises once it gets passed to SparseArray.  The actual bug this fixes is the `fill_value = func(...)` on 471.
thanks
REF: check can_hold_element instead of try/except
State: closed

`_can_hold_element` is not well-documented, and in local testing it does _not_ match my intution of "we could do `self.values[:] = element` without raising".  But adding two assertions in `fillna` _does_ work in all the tests:

```
         # fillna, but if we cannot coerce, then try again as an ObjectBlock
        try:
            # equivalent: self._try_coerce_args(value) would not raise
            # Note: we only call try_coerce_args to let it raise
            self._try_coerce_args(value)
            assert self._can_hold_element(value)
        except (TypeError, ValueError): 
            assert not self._can_hold_element(value)
```

(at least after patching the `DatetimeBlock` and `TimedeltaBlock` implementations)

Labels:
name: Internals
description: Related to non-user accessible pandas implementation
name: Clean

Comments:
if you can update to those comments, otherwise lgtm.
Follow up coming shortly, let’s make elif change there
@jreback gentle ping for this and #27229; next-steps in the works for both of these
thanks @jbrockmendel if you can do those followups in next PRs
CLN: Remove unused vars in roll_window
State: closed


Labels:
name: Clean
name: Window
description: rolling, ewma, expanding

Comments:
lgtm - nice catch! Is there any way to auto-detect this in Cython? Vaguely recall trying but there being some linting limitation @jbrockmendel 
We're not going to get flake8 working on the cython files anytime soon, so maybe we can get gcc/clang/whatever to issue a warning about declared-but-unused variables?
thanks @ihsansecer
BUG: Incorrect Message in KeyError with MultiIndex
State: closed

- [x] closes #27250
- [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: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Error Reporting
description: Incorrect or improved errors from pandas
name: MultiIndex

Comments:
thanks @simonjayhawkins
Raise with message for 3.5.2 and earlier
State: closed

Closes https://github.com/pandas-dev/pandas/issues/27247



Labels:
name: Build
description: Library building on various platforms

Comments:
Would it work to put this error in setup.py instead? Install time errors seem better than import time errors. Or are there cases where that doesn’t get run?
> Would it work to put this error in setup.py instead? Install time errors seem better than import time errors. Or are there cases where that doesn’t get run?

i think if you have a old pip and install from a wheel this wont be hit in setup.py
only at import time
Agreed that this should be in `setup.py`, but after thinking about it more, I'm hesitant to duplicate things that pip is doing (when used correctly).

The reported fail case in https://github.com/pandas-dev/pandas/issues/27247 was from (incorrectly) using `python setup.py develop` directly. I'd rather educate users on how pip says it should be done, rather than add duplicative checks.
BUG: Fix CategoricalIndex.__contains__ with non-hashable, closes #21729
State: closed

- [x] closes #21729
- [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: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Categorical
description: Categorical Data Type

Comments:
thanks @jbrockmendel liking your pushing small orthogonal patches.
TST/STYLE: concatenate string literals post black reformatting
State: closed


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

Comments:
do these pass line length? you might need to use parens and split on lines 
> do these pass line length? you might need to use parens and split on lines

should pass black lint rule. is that what you mean?
is the flake rule in setup.cfg updated? to 79
> is the flake rule in setup.cfg updated? to 79

am I missing something? we're using 88 and black ignores that sometimes anyway.
actually we r using the black default which is 88
so ok
thanks @simonjayhawkins
DOC: cleanup docstring for read_json and fix error in contribution guide
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

- Contribution Guide
    - fix incorrect paths to doc folder
- read_json()
    - add period to end of sentences
    - conform parameter types to project standard
    - add newlines for readability and consistency
    - for convert_dates parameter, fix typos and malformed description and add list as accepted type
    - make Returns section conform to project standard

Labels:
name: Docs

Comments:
cc @datapythonista 
* New changes to read_json docstring in `a92642d`:
    * Remove colon after `default` for `path_or_buf` parameter
    * Add default value for `orient` parameter (addresses hanging comma, see comment above)
    * Fix error on `typ` parameter by adding set of accepted values and a description
    * Fix error in Returns section by moving description to second line (see comment above)
    * Fix error in See Also section by adding description to DataFrame.to_json (see comment above)
    * Add Series.to_json to See Also section
Issue has been opened. #27308 

And the fixes to the type hints have been made.
can you merge master
Done.
Thanks @mpmoran!
DOC: Explicitly include "private" ExtensionArray methods in API docs
State: closed

- [x] closes #24067
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`


Labels:
name: Docs
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
cc @jorisvandenbossche @TomAugspurger 
@tadeja can you take a look at the built docs at http://pandas-docs.github.io/pandas-docs-travis/reference/api/pandas.api.extensions.ExtensionArray.html#pandas.api.extensions.ExtensionArray when the CI build on master for this finishes?
Thanks! It looks good over here:
https://dev.pandas.io/reference/api/pandas.api.extensions.ExtensionArray.html#pandas.api.extensions.ExtensionArray
(Date: Jul 16, 2019 Version: 0.25.0rc0+76.g26bd34df2)

... while this one appears unchanged and I am just wondering if that's going to be OK:
http://pandas-docs.github.io/pandas-docs-travis/reference/api/pandas.api.extensions.ExtensionArray.html#pandas.api.extensions.ExtensionArray
(Date: Jun 18, 2019 Version: 0.25.0.dev0+752.g49f33f0d)

I don't think the second one is being updated anymore.

On Tue, Jul 16, 2019 at 4:58 PM tadeja <notifications@github.com> wrote:

> Thanks! It looks good over here:
>
> https://dev.pandas.io/reference/api/pandas.api.extensions.ExtensionArray.html#pandas.api.extensions.ExtensionArray
> (Date: Jul 16, 2019 Version: 0.25.0rc0+76.g26bd34df2)
>
> ... while this one appears unchanged and I am just wondering if that's
> going to be OK:
>
> http://pandas-docs.github.io/pandas-docs-travis/reference/api/pandas.api.extensions.ExtensionArray.html#pandas.api.extensions.ExtensionArray
> (Date: Jun 18, 2019 Version: 0.25.0.dev0+752.g49f33f0d)
>
> —
> You are receiving this because you modified the open/close state.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/27279?email_source=notifications&email_token=AAKAOIV6DZCZEPF2HBJ636LP7ZABZA5CNFSM4H6WHX62YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2CI7PQ#issuecomment-512004030>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAKAOISXWBWS2OM5BJC2VNLP7ZABZANCNFSM4H6WHX6Q>
> .
>
BUG: Fix+test division by negative zero
State: closed

Discovered while trying to reconcile Series vs Index arithmetic behavior.

A couple of other small misc changes ported from other branches, will be noted inline.

Labels:
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

Comments:
is this change user facing at all? or just test comparisons?
> is this change user facing at all? or just test comparisons?

Yes, just barely:

```
np.random.seed(1)
idx = pd.Index(np.random.randn(5))

>>> idx // (idx * 0)

# master
Float64Index([inf, -inf, -inf, -inf, inf], dtype='float64')

# PR
Float64Index([inf, inf, inf, inf, inf], dtype='float64')
```

Similar for divmod.  truediv is unchanged.  Series is unchanged.
> > is this change user facing at all? or just test comparisons?
> 
> Yes, just barely:
> 
> ```
> np.random.seed(1)
> idx = pd.Index(np.random.randn(5))
> 
> >>> idx // (idx * 0)
> 
> # master
> Float64Index([inf, -inf, -inf, -inf, inf], dtype='float64')
> 
> # PR
> Float64Index([inf, inf, inf, inf, inf], dtype='float64')
> ```
> 
> Similar for divmod. truediv is unchanged. Series is unchanged.

ok that's fine then, wouldn't bother with a release note.
thanks @jbrockmendel
ENH: Raise ValueError for unsupported Window functions
State: closed

- [x] ref #26597
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Weighted std could be implemented but there are also other functions weighted `Window` is lacking.

Labels:
name: Window
description: rolling, ewma, expanding

Comments:
@jreback it is ok when directly calling an unsupported function but raised error (`AttributeError: 'Window' object has no attribute 'dtype'`) is not informative for `agg` function
Please don't merge this as it raises `ValueError: no results` in some cases. Will check it when I have time
`skew` and `kurt` functions as an argument to `agg` raises `unknown string function` error. `min` and `max` functions on the other hand doesn't raise anything and returns `Window`object.
lgtm @ihsansecer ping on green.
thanks @ihsansecer 
btw now that pandas/tests/window/ exists would love to split test_window even more :->
@jreback noted :)