BrandonFors/pandas-github-issues huggingface dataset:
BUG: support min/max functions for rolling windows with custom BaseIndexer
State: closed

- [X] xref #32865 
- [X] 1 tests added / 1 passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry

## The problem
We currently don't support several rolling window functions when building a rolling window object using a custom class descended from `pandas.api.indexers.Baseindexer`. The implementations were written with backward-looking windows in mind, and this led to these functions breaking.
Currently, using these functions returns a `NotImplemented` error thanks to #33057, but ideally we want to update the implementations, so that they will work without a performance hit. This is what I aim to do over a series of PRs.

## Scope of PR
This PR proposes an alternative implementation for the `min` and `max` rolling window functions implemented in the `_roll_min_max_variable` function in the `aggregations.pyx` file.

## Perf note
This implementation is slightly faster than the one on master branch (tested with a `Series` with 10e6 randomly generated values and `timeit`). If necessary, I can add a benchmark to the benchmark suite, although it must be noted that we need to benchmark with large arrays of data to minimize the effect of overhead.

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

Comments:
The core idea is that we ancor the output index we write at to the index in the input value array it corresponds to. This generalizes the function and also removes the need to fiddle with `close_offset`.

Aside from that, I reuse the original algorithm with some minor adjustments that allow the functions to work with any monotonic windows (window starts and ends stay in place or move to the right).
On another note, with these changes, it is no longer necessary to have separate code for initializing the first value and for writing the final value. Everything can be done in the main loop.
@jreback 
Between the new issue and the changes made, I think all the comments are covered. Is there anything else we should do before merging?
 will looks again soon
Changed the `ForwardIndexer` implementation to mirroring the one I will propose exposing to users in a separate PR.
Some pipelines are canceling on timeout. Should have nothing to do with this PR, but I restarted the tests just in case.
Hm. py37_np_dev seems to be hanging for everyone. If it doesn't recover after an hour, I'll post an issue (unless someone beats me to it).
thanks @AlexKirko 
Thanks!
DOC: Fixed examples in pandas/core/arrays/
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: Docs

Comments:
LGTM
Thanks @MomIsBestFriend
WEB: Moving pandas blog to the website
State: closed

Moving the pandas blog posts (now in a separate repo/project) to the `web/community/blog/` in our website. The static site generator we're using for the web, renders them automatically using our layout:

![pandas_blog_2](https://user-images.githubusercontent.com/10058240/78026717-01a10b80-7354-11ea-830e-65af77ed4838.png)

The posts list doesn't have much visual change, just links to the new path:

![pandas_blog_1](https://user-images.githubusercontent.com/10058240/78026785-1b425300-7354-11ea-8cdd-ab632b62d41e.png)

The main change here, besides adding the posts markdown files (and images) is that the static site generator is now able to not only fetch from rss, but also from the file system.

CC: @TomAugspurger 

Btw @TomAugspurger , I increased the number of posts we show in the list, and see that there are some about dask-ml and other things unrelated to pandas being fetched from your blog. Would be great if you can add a pandas tag to your blog, so we only fetch the relevant content.

Labels:
name: Web
description: pandas website

Comments:
Hello @datapythonista! 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-31 13:29:14 UTC
I'll try to get my tags updated today.
Merging this today if there are no objections.
Issue #33161 - Use empty_frame fixture
State: closed

Replaced Empty data frames with fixture empty_frame()

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:

Comments:
Unfortunately looks like another user claimed this first and submitted a PR in #33167 , so sticking with that. Thanks for the PR though - we have plenty more issues to tackle and would welcome contributions on plenty more
DOC: Fix examples in pandas/core/ops/
State: closed

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


Labels:
name: Docs

Comments:
> lgtm, thanks @MomIsBestFriend 
> 
> I think in a follow up would be good to have a single call for all the doctests. Not sure why we group few cases, then call some files individually...
> 
> Also, if you keep fixing tests, may be you can fix one directory at a time. So, for example, instead of having to call evey file in `pandas/core/arrays` separetely, we can just have one call for the whole directory.
> 
> Ideally, at some point we simply want to have a call to `pytest -q --doctest-modules pandas`
> 
> Thanks!

@datapythonista Exactly, this is what I am aiming for.

And about the reason why there are "groupings" of, some of them are there for legacy reasons and the other reason is to have it as a "placeholder", I was afraid that even if a file is fixed, overtime it will become "wrong" agian, so this is just putting things in the "safe" place.
ping @simonjayhawkins 
Thanks @MomIsBestFriend
CI: add doctest check for done modules
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: Docs
name: CI
description: Continuous Integration

Comments:
thanks @MomIsBestFriend
CLN: using C-API of datetime
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: Clean

Comments:
lgtm. @jbrockmendel over to you.
thanks @MomIsBestFriend
CLN: Added static types
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

---

I'll revert any change related to styling, I applied "black" formatting in some places, just so I could think straight.

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

Comments:
lgtm
@jreback any outstanding comments?
thanks @MomIsBestFriend
CLN: Replace DataFrame() with empty_frame in tests
State: closed

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


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

Comments:
Hello @Jason-M-Chan! 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 03:51:38 UTC
@Jason-M-Chan Thank you for taking on this!

---

My suggestion is to have smaller PRs, that way it will be easier to debug if necessary, and to avoid future merge conflicts.
I made the changes recommended above, but the Azure Pipeline still gives errors saying

`TypeError: setup_method() missing 1 required positional argument: 'empty_frame'`

I'm not sure how to debug this. Also, sorry about the large pull request, I will make smaller ones.
-1 on this.  there are good use cases for fixtures, this is not one  of them.
@jbrockmendel can you clarify your objection? I think this helps make what we consider an `empty_frame` consistent across the code base
> can you clarify your objection? I think this helps make what we consider an empty_frame consistent across the code base

1) as you mentioned in another discussion, copy/paste-ability is desirable.  fixtures reduce copy/paste-ability, so should be used only in use cases where they provide non-trivial de-duplication or parametrization
2) When i look at `DataFrame()` i know exactly what im dealing with.  with a fixture, i have to go look that up.
3) singleton fixtures make test signatures harder to grok.  tz_naive_fixture is useful, empty_frame is not.

cc @jorisvandenbossche
closing as stale. @WillAyd feel free to reopen if you want to pursue this.
Json parametrize more2
State: closed


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: IO JSON
description: read_json, to_json, json_normalize

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







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

##### Comment last updated at 2020-03-31 03:24:44 UTC
LGTM
@jreback no to both comments - these are the only fixtures with those names
> @jreback no to both comments - these are the only fixtures with those names

kk
BUG: isna_old with td64, dt64tz, period
State: closed

- [ ] closes #xxxx
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Started as a CLN branch and i left that in for now, can separate if desired.  the bugfix is on L287.

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

Comments:
thxs
REF: PeriodIndex test_indexing tests
State: closed

This should be it for the PeriodIndex tests for now.

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

Comments:
if you want to write down some general policies for where we locate tests would be great,

e.g. testing_indexing includes where, contains, but not monotonic for example (of course this might not be so consistent yet)
DOC: Fix capitalization among headings in documentation files (#32550)
State: closed

Headers updated for the following files:

```
- [x] doc/source/user_guide/options.rst
- [x] doc/source/user_guide/reshaping.rst
- [x] doc/source/user_guide/text.rst
- [x] doc/source/user_guide/timeseries.rst
- [x] doc/source/user_guide/visualization.rst
```

Other files updated in #32944, #32978, #32991  and #33147


Labels:
name: Docs

Comments:
Thanks @themien
DOC: Fix capitalization among headings in documentation files (#32550)
State: closed

Headers updated for the following files:

```
- [x] doc/source/user_guide/categorical.rst
- [x] doc/source/user_guide/computation.rst
- [x] doc/source/user_guide/cookbook.rst
- [x] doc/source/user_guide/gotchas.rst
- [x] doc/source/user_guide/groupby.rst
```

Other files updated in #32944, #32978 , #32991 and #33149

Labels:
name: Docs

Comments:
Thanks @themien
add match message for pytest.raises()
State: closed

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


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

Comments:
I'm not sure if I have broken something in the tests, the failures are from the modules which I haven't touched, waiting for the fixes in master.
LGTM
@jreback should be good to merge now.
thanks @sathyz
STY: Using __all__; removed "noqa" comment
State: closed

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


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

Comments:
I personally don't have a strong opinion on relative vs absolute imports, just wanted to remove the ```# flake8: noqa``` comment.

---

If we do want relative imports in some places, https://dev.pandas.io/docs/development/code_style.html#imports-aim-for-absolute should be updated.
> I find the relative imports much clearer to signal it are all imports internally from the module.

I'm with Joris on this one.
absolute imports are recommended by PEP8 and are used 99% of the time in our code base, so I think we should go with this
PEP8 also says:

> However, explicit relative imports are an acceptable alternative to absolute imports, especially when dealing with complex package layouts where using absolute imports would be unnecessarily verbose

(which I would say applies here)

So I don't think PEP8 is a good reason to do this. 
Consistency within pandas of course *is*. And it is correct than almost all code uses absolute imports. But that is also because there are regularly PRs like this one that converts to absolute imports that we merge ;) (just meaning, this is a "rule" we set ourselves that can be discussed)

> I am personally -1 on this, I find the relative imports much clearer to signal it are all imports internally from the module.

I disagree, we barely use them. having a mix is SO confusing. Since we use absolute almost everywhere we should just continue to do this.
all that said, we could have a policy to use relative in *some* parts of the codebase. I just don't want to mix it *within* a file. This file looks fine as is. But we need to define how we communicate this policy (and how its enforced). 
Is there a path forward here? I'm still OK with this
> I personally don't have a strong opinion on relative vs absolute imports, just wanted to remove the # flake8: noqa comment.

Let's just do the `__all__` for now
Thanks @MomIsBestFriend
CLN: unused import  _libs/reduction + remove "noqa" comment in _libs/__init__.py
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: Clean

Comments:
Thanks @MomIsBestFriend
BUG: to_sql no longer raises an AttributeError when saving an OBB date
State: closed

- [x] closes #26761
- [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: IO SQL
description: to_sql, read_sql, read_sql_query

Comments:
So does this fully close the associated issue? Might be overlooking but I see the OutofBoundsError in the issue which isn't checked for here
The `OutofBoundsError` is no longer the issue since I think an Index with `object` dtype is created to hold the OOB date. The new issue is that the Index with `object` dtype raises an `AttributeError` since it doesn't have a `tz` attribute
REF: de-curry unstack functions
State: closed

I find this much easier to follow without the double-partial.  We also avoid re-doing some work, though AFAICT its pretty tiny perf-wise.

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

Comments:
yep looks better. I think has some intentions a while back to pass generic reshapers around, but didn't happen.

If you can type things in a followup would be great.
BUG: Fix SeriesGroupBy.quantile for nullable integers
State: closed

- [x] closes #33136
- [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: Groupby
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
Great thanks @dsaxton
BUG: DataFrame.resample raised AmbiguousTimeError at a midnight DST transition
State: closed

- [x] closes #25758
- [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: Timezones
description: Timezone data dtype
name: Resample
description: resample method

Comments:
Going to merge, but let me know if there is any followup I should do.
BUG: create new MI from MultiIndex._get_level_values
State: closed

- [x] closes #33131
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Closes #33131, a weakref was released too early.

Labels:
name: Bug
name: Period
description: Period data type
name: MultiIndex

Comments:
If PeriodEngine.get_indexer is ever called it will raise AttributeError, ditto get_pad_indexer, get_backfill indexer.  
@jreback , this is a regression since v1.0, causes by #32669, so a whatsnew isn't needed here.

@jbrockmendel , it looks like it also did that in v1.0?:

```python
>>> idx = pd.MultiIndex.from_arrays([pd.PeriodIndex([pd.Period("2019Q1"), pd.Period("2019Q2")], name='b')])
>>> idx2 = pd.MultiIndex.from_arrays([idx._get_level_values(level) for level in range(idx.nlevels)])
>>> idx2.levels[0]._engine.get_indexer(idx.levels[0])
AttributeError: 'super' object has no attribute '_ensure_mapping_populated'  # both 1.0 and master
```

thanks @topper-123
BUG: to_datetime with infer_datetime_format dropped timezone names
State: closed

- [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: Timezones
description: Timezone data dtype

Comments:
LGTM pending GH ref
thanks @mroeschke
CLN: Added static types
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: Clean

Comments:
LGTM
@jreback here as well; I think good to merge
thanks @MomIsBestFriend
REF: reshape.concat operate on arrays, not SingleBlockManagers
State: closed

cc @TomAugspurger is there a better way to handle the assert_series_equal patch for the PandasArray tests?

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

Comments:
> Can you explain the issue your change to assert_series_equal is working around?

Without the patched assert_series_equal, we get

```
_________________________________________________________________________ TestGetitem.test_loc_iloc_frame_single_dtype[object] _________________________________________________________________________

self = <pandas.tests.extension.test_numpy.TestGetitem object at 0x127f29f90>
data = <PandasArray>
[ (0,),  (1,),  (2,),  (3,),  (4,),  (5,),  (6,),  (7,),  (8,),  (9,), (10,),
 (11,), (12,), (13,), (14,...(87,),
 (88,), (89,), (90,), (91,), (92,), (93,), (94,), (95,), (96,), (97,), (98,),
 (99,)]
Length: 100, dtype: object

    def test_loc_iloc_frame_single_dtype(self, data):
        # GH#27110 bug in ExtensionBlock.iget caused df.iloc[n] to incorrectly
        #  return a scalar
        df = pd.DataFrame({"A": data})
        expected = pd.Series([data[2]], index=["A"], name=2, dtype=data.dtype)
    
        result = df.loc[2]
>       self.assert_series_equal(result, expected)

pandas/tests/extension/base/getitem.py:81: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

cls = <class 'pandas.tests.extension.test_numpy.TestGetitem'>, left = A    (2,)
Name: 2, dtype: object, right = A    (2,)
Name: 2, dtype: object, args = (), kwargs = {}

    @classmethod
    def assert_series_equal(cls, left, right, *args, **kwargs):
>       return tm.assert_series_equal(left, right, *args, **kwargs)
E       AssertionError: Attributes of Series are different
E       
E       Attribute "dtype" are different
E       [left]:  object
E       [right]: object

pandas/tests/extension/base/base.py:13: AssertionError
```

`test_loc_iloc_frame_single_dtype` is currently xfailed.  The edit to `PandasArray.astype` was because this had caused one of the cases of `test_loc_iloc_frame_single_dtype` to XPASS.  Removing the xfail led to 9 failures, 8 of which were fixed by the edit to `PandasArray.astype`.

I think the assert_series_equal issue has to do with the way we are patching `PandasArray._typ` in these tests, but i havent chased it down to be sure.
Updated to just xfail more selectively and avoid patching assert_series_equal
Can you check https://github.com/pandas-dev/pandas/pull/33125/files#r400169295?
DOC: Updating capitalization of  doc/source/development
State: closed

Regarding issue #32550. Changes to documentation folder doc/source/development to capitalise title strings and keep keyword exceptions as is

Labels:
name: Docs

Comments:
Thanks @cleconte987. Most of this is duplicated of #32944, we're going to merge that one, so you may want to revert those files. Also, try to coordinate with @themien who is also working on this.

For the titles that are like `:class:...` we can't apply your changes, that is a special encoding to automatically create links. What we'll have to do is to update the script to skip titles that start with `:`.
> Thanks @cleconte987. Most of this is duplicated of #32944, we're going to merge that one, so you may want to revert those files. Also, try to coordinate with @themien who is also working on this.
> 
> For the titles that are like `:class:...` we can't apply your changes, that is a special encoding to automatically create links. What we'll have to do is to update the script to skip titles that start with `:`.

Ok I will follow what @themien is doing.
I modify the script as you said
And I update PR soon
@cleconte987 can you merge upstream master into your branch to resolve merge conflicts. see https://pandas.pydata.org/docs/development/contributing.html#updating-your-pull-request
Hello @cleconte987! 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 21:13:35 UTC
Thanks for working on this @cleconte987 
You're welcome!
Fix ambiguous reference to "previous" section
State: closed

This section originally came just after "10 minutes to pandas",
but now it's after the "Getting started tutorials".
Also, the examples have diverged a bit, so let's not pretend
that we are creating the same objects.

- [ ] 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
CLN: update Appender to doc decorator with case __doc__
State: closed

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


Labels:
name: Docs
name: Clean

Comments:
Thanks @HH-MWB