BrandonFors/pandas-github-issues huggingface dataset:
DOC: clarify boxplot whiskers extension
State: closed

As I'm getting started with box plots, I found confusing that the `DataFrame.boxplot` docstring didn't mention that whiskers extend _at most_ to 1.5 * IQR away from the box's edges.

- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry -> is it required for such a minor change ?


Labels:
name: Docs

Comments:
This doc change is sorely needed. Thanks for doing it! However, your explanation is wrong. The whiskers only ever extend to 1.5*IQR if there happens to be a data point exactly there. Here's how it actually works: https://stackoverflow.com/a/49781675/4691830 .

I suggest the following wording:

``By default, they extend no more than `1.5 * IQR (IQR = Q3 - Q1)` from the edges of the box, ending at the farthest data point within that interval. Outliers are plotted as separate dots.``


Thanks @joooeey for clarifying this, I applied your suggestion !
>  However, your explanation is wrong

@joooeey are you sure it's wrong? When the OP wrote

> they extend up to `1.5 * IQR (IQR = Q3 - Q1)` from the edges of the box, or to the farthest
data point if it lies within this interval

I think they meant that the whiskers only extend there if there's a point there, which I think is what you're saying.

> I suggest the following wording:

Having said that, thanks for this, I do think your wording is very clear
@neutrinoceros did you push it? :) I don't (yet) see anything here
> I think they meant that the whiskers only extend there if there's a point there, which I think is what you're saying.

I believe my own wording was correct if you read the "or" _and_ the "within that interval" as an _inclusive_, which showed my own misunderstanding.

> @neutrinoceros did you push it? :) I don't (yet) see anything here

Oops, you're right, the push failed for some reason.
Great thanks @neutrinoceros
ENH: Implement groupby.sample
State: closed

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


Labels:
name: Enhancement
name: Groupby

Comments:
thanks @dsaxton very nice!
BUG : Series.to_timestamp and Series.to_period raise user-facing AssertionError
State: closed

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


Labels:
name: Datetime
description: Datetime data dtype
name: Error Reporting
description: Incorrect or improved errors from pandas
name: Period
description: Period data type

Comments:
Thanks @vipulrai91 for the PR. can you add tests that test for the error message.
Also a whatsnew in doc/source/whatsnew/v1.1.0rst
> Also a whatsnew in doc/source/whatsnew/v1.1.0rst

Yes , I  would add it once the above part is fixed.
Hello @vipulrai91! 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-05-20 11:42:15 UTC
> Should the test be like this

we use `with pytest.raises(..., match=...)` pattern

> pandas/tests/series/test_period.py
 
pandas\tests\series\methods\test_to_period.py and pandas\tests\series\methods\test_to_timestamp.py


> > Also a whatsnew in doc/source/whatsnew/v1.1.0rst
> 
> Yes , I would add it once the above part is fixed.

add in Backwards incompatible API changes section

As this previously raised AttributeError, https://github.com/pandas-dev/pandas/issues/33327#issuecomment-625849949, can add similar to existing release note for swaplevels

```
- :meth:`DataFrame.swaplevels` now raises a  ``TypeError`` if the axis is not a :class:`MultiIndex`.
  Previously an ``AttributeError`` was raised (:issue:`31126`)
```
> Thanks @vipulrai91 for the PR. can you add tests that test for the error message.



> can you add a test that hits this

@jreback 
 added tests  at  : pandas/tests/series/methods/test_to_period.py and
       pandas/tests/series/methods/test_to_timestamp.py

Please let me know if this is correct , also , I am not sure which parts to add the doc in doc/source/whatsnew/v1.1.0.rst
> I am not sure which parts to add the doc in doc/source/whatsnew/v1.1.0.rst

see https://github.com/pandas-dev/pandas/pull/34067#issuecomment-627342733

thanks @vipulrai91 very nice
DOC: use the sphinx-panels extension for cards instead of raw html
State: closed

This is an alternative for https://github.com/pandas-dev/pandas/pull/32065, using the https://sphinx-panels.readthedocs.io/en/latest/ extension (which was created inspired on how we are using this in our docs, but then nicely packaged in a sphinx extension to avoid the use of raw html).

Labels:
name: Docs

Comments:
@jorisvandenbossche still relevant?
@jorisvandenbossche gentle ping
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.
@jorisvandenbossche ?
@jorisvandenbossche pls update or close
@dsaxton I updated this PR to also convert the front page cards to use sphinx-panels, and with some custom styling to keep the same look and feel, I get this:

![image](https://user-images.githubusercontent.com/1020496/112748104-f65ddb80-8fb9-11eb-9ce4-df1b29902040.png)

The "buttons" are using the default darker grey and sharper corners from sphinx-panels, but if we prefer the lighter gray / more rounded corners as it is now, that's easy to change as well. 

For reference, screenshot of the current https://pandas.pydata.org/docs/dev/:

![image](https://user-images.githubusercontent.com/1020496/112748135-32913c00-8fba-11eb-821f-ec9e1932232e.png)

> @dsaxton I updated this PR to also convert the front page cards to use sphinx-panels, and with some custom styling to keep the same look and feel, I get this:

Nice. The stronger contrast with the dark button color does make them a bit more readable. This is subjective though so I'm curious how others feel.
+1 on the look and not using raw html
DOC:GH34026
State: closed

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


Labels:
name: Docs

Comments:
Hello @moaraccounts! 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-05-15 18:27:11 UTC
EDIT: <b>headdesk</b>. I was on the wrong branch. Never mind. Should be good to go.


I'm not sure what I'm doing wrong. I'm following [this guide](https://pandas.pydata.org/docs/dev/development/contributing.html#updating-your-pull-request)
I just fixed the white space issues, then manually ran code_checks.sh for docstrings, lint, and patterns. I'm attaching those three outputs. I also ran `git diff upstream/master -u -- "*.py" | flake8 --diff` with no issues.

When I run `git push origin <my-branch>`, I receive `Everything up-to-date`. For some reason, the CI checks are still failing. Any suggestions?

[check_docstrings.txt](https://github.com/pandas-dev/pandas/files/4600404/check_docstrings.txt)
[check_lint.txt](https://github.com/pandas-dev/pandas/files/4600405/check_lint.txt)
[check_patterns.txt](https://github.com/pandas-dev/pandas/files/4600406/check_patterns.txt)



Thanks @moaraccounts 

CI failures are unrelated

Did you try building the documentation locally? https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#contributing-to-the-documentation
> Thanks @moaraccounts
> 
> CI failures are unrelated
> 
> Did you try building the documentation locally? https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#contributing-to-the-documentation

@MarcoGorelli Yes indeed. I can build the documentation locally and see my changes. Attached are the screenshots of rstrip and lstrip. Do those help?
![lstrip](https://user-images.githubusercontent.com/9169730/81846505-33df8480-9520-11ea-9d5d-54a96b72a48f.png)
![rstrip](https://user-images.githubusercontent.com/9169730/81846518-393ccf00-9520-11ea-8946-9ab3bc0f66d3.png)

@moaraccounts mind if I ask why you closed this?
Ooh, I thought since the changes were approved I could close it! My bad! I will reopen.
It's OK :) A core dev will review this eventually (please be patient though, there's a long queue) and merge it
Awesome! Ok, no worries on the waiting. Thank you!
> CI failures are unrelated

My bad, CI failures _are_ related, you need to get 

```bash
python scripts/validate_docstrings.py pandas.Series.str.lstrip
```
and
```bash
python scripts/validate_docstrings.py pandas.Series.str.rstrip
```
to pass

EDIT
----
```bash
python scripts/validate_docstrings.py pandas.Series.str.strip
```
too
Thanks @moaraccounts! Nice contribution 
Thank you!
BUG: Make nullable booleans numeric
State: closed

- [x] closes #34051
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Also a decent speedup since this is now going through Cython space:
```python
df = pd.DataFrame({"a": [1] * 50_000 + [2] * 50_000, "b": pd.array([True] * 100_000)})

# master
%timeit df.groupby("a").sum()                                                                                                                                                                  
# 5.15 ms ± 18 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

# branch
%timeit df.groupby("a").sum()                                                                                                                                                                  
# 1.94 ms ± 10.8 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
```

Labels:
name: Groupby
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations
name: NA - MaskedArrays
description: Related to pd.NA and nullable extension arrays

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-05-10 15:59:48 UTC
thanks
more informative error message with np.min or np.max on unordered Categorical
State: closed

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


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

Comments:
I think all comments addressed. will merge tomorrow if no further comments.
CI: check_freq=False
State: closed

Aimed at a test that has been periodically failing in the CI for a week or so.

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

Comments:
looks ok, comments by @simonjayhawkins 
comments addressed
green
k thanks. pls open an issue to change this to `check_freq=False`; I *think* this might break downstream, IIRC a comment by @jorisvandenbossche 
> k thanks. pls open an issue to change this to `check_freq=False`; I _think_ this might break downstream, IIRC a comment by @jorisvandenbossche

Jepp, this broke some code for me. Will open the mentioned issue.
Bug in Series.groupby would raise ValueError when grouping by PeriodIndex level
State: closed

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


Labels:
name: Groupby
name: Period
description: Period data type

Comments:
I think all comments addressed. will merge tomorrow if no further comments.
Opened https://github.com/pandas-dev/pandas/issues/34240 for the followup. LGTM.
Bug in DataFrame.replace casts columns to ``object`` dtype if items in ``to_replace`` not in values
State: closed

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


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

Comments:
I am ok with reverting things for the 1.0.4 branch but not for master.
thanks @simonjayhawkins
REF: use unpack_zerodim_and_defer on EA methods
State: closed


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

Comments:
@jbrockmendel I am +1 on this, can you merge master. If @jorisvandenbossche object to the array/base.py impl then ok but otherwise lgtm.
can u rebase and add the suggested test
Mothballing
I reopened this because I think it are useful changes. I only changed it back in the base implementation (which is not used in pandas), as I suggested above. 
@jbrockmendel are you fine with merging it like this?
@jorisvandenbossche i just rebased and force-pushed, might have undone the commit you pushed.  feel free to re-do that
I forced pushed again my branch, but it still needs a merge of master.
And also merged master
lgtm. @jorisvandenbossche merge when ready. (or ping and i will)
CLN: spelling fixes in docstrings
State: closed

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


Labels:
name: Clean

Comments:
Appears that `TestHDFStore.test_append_with_data_columns` is failing in 1 of the 5 TravisCI builds. 

I didn't change that function, and it looks to have passed in all other Azure pipeline builds. Is this a known flaky test?
> Is this a known flaky test?

yes.
Thanks @akosfurton! The failing Travis build was unrelated test failure
TST: Add Series.update ExtensionArray tests
State: closed

cc @simonjayhawkins 

xref https://github.com/pandas-dev/pandas/issues/25744 https://github.com/pandas-dev/pandas/issues/34020

I think the dtype issue for DataFrame.update with categorical columns mentioned in the first issue is still unaddressed, so likely don't want to only close and lose that (perhaps break into its own issue)?

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
> I think the dtype issue for DataFrame.update with categorical columns mentioned in the first issue is still unaddressed, so likely don't want to only close and lose that (perhaps break into its own issue)?

sgtm.

although this PR _could_ close the 2 issues (with the caveat of opening a new issue for the DataFrame case), my preference would be to replace test_update_extension_array_series with a test in tests/extension instead.


> my preference would be to replace test_update_extension_array_series with a test in tests/extension instead.

I think this is okay since it's the Series update method that we're testing? TBH the extension-specific tests are a bit confusing to me because of the class hierarchies and heavy use of fixtures (IMHO it's easier if we can reason about tests locally)
> lgtm. ping on green.

@jreback ping
thanks
TST: query with timezone aware index & column
State: closed

- [X] closes #29463
- [X] tests added / passed
- [X] 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
name: Timezones
description: Timezone data dtype

Comments:
> LGTM. If you could merge master that will fix the failing CI

Thanks. Merging now.
> > LGTM. If you could merge master that will fix the failing CI
> 
> Thanks. Merging now.

 @mroeschke  All checks passes.

> Sorry this PR hasn't been merged yet. Mind merging master into this branch?
> 
> After that, we can probably merge this in

@mroeschke  done.
Thanks for your patience @vipulrai91. Nice contribution!
> Thanks for your patience @vipulrai91. Nice contribution!

Thank you @mroeschke @MarcoGorelli @jreback  for the help.
Looking forward to more contributions.
ENH: Add compression to stata exporters
State: closed

Add standard compression optons to stata exporters

closes #26599

- [X] closes #26599
- [X] tests added / passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Enhancement
name: IO Stata
description: read_stata, to_stata

Comments:
Thanks @bashtage for the PR. if you merge master the np-dev ci failure should be resolved. Also a couple of lint failures.
can you merge master
thanks @bashtage
BUG: groupby with as_index=False shouldn't modify grouping columns
State: closed

- [x] closes #32579
- [x] closes #21090
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

This makes any groupby function that uses _GroupBy._make_wrapper to act on self._obj_with_exclusions rather than self._selected_obj, in parallel with the cython paths. Also similar to the cython paths, the grouping columns are added back onto the result in _wrap_applied_output. Similar remarks apply to nunique, which does not go through the _make_wrapper.

After finishing this, I found PR #29131. This PR does not change the behavior of calling apply() itself, only the previous code paths that would internally go through apply. Also, the change made there does not have any impact when as_index is False.

Labels:
name: Groupby
name: API - Consistency
description: Internal Consistency of API/Behavior

Comments:
@jreback Changes made, checks are green.
@jreback changes made and green.
@jreback Changes made and tests pass. It seemed best to me to add a new api breaking section for nunique when as_index=True. While the code changes to nunique were the same for both issues, from an api standpoint I think they are different cases. If you'd like me to combine into one section though, I can do that.
thanks @rhshadrach fixing groupby bugs sometimes are non-trivial in time, thanks for sticking with it and keep em coming!
REF: avoid importing pd.NA in tslibs
State: closed

The existing cimport of pd.NA into tslibs.nattype is a circular dependency (its not actually clear to me why it doesnt raise).  I'm finding that its presence is making other refactoring around tslibs.offsets more difficult.  This PR moves the import of pd.NA to tslib, breaking the circular dependency.

This _does_ change some behavior, but none that is tested, so we should discuss what places we want to recognize pd.NA.  In particular, `pd.Timedelta(pd.NA)` returns `NaT` in master and raises `ValueError` in this PR.  `pd.Timestamp(pd.NA)` and `pd.Period(pd.NA)` raise TypeError and ValueError, respectively, in both master and this PR.

cc @jorisvandenbossche what is your preferred behavior?

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

Comments:
This keeps importing pd.NA into tslibs, though? (but in tslibs.pyx itself it is less a problem?)

The behaviour change is certainly fine for me. I think it is even better than we raise for pd.NA for now (since pd.NA is not yet used in any datetimelike dtype. 
> This keeps importing pd.NA into tslibs, though? (but in tslibs.pyx itself it is less a problem?)

tslib.pyx, but not the tslibs directory, right.  Since nothing inside tslibs/ depends on tslib.pyx, it is not that big a deal.
Ah, I missed the tslib.pyx vs tslib directory
can you merge master. do we have sufficient test for this change?
We could in principle test `pd.Timedelta(pd.NA)`, but I also don't think that's very important, as I would regard that behaviour experimental / subject to change.
CLN: remove unused out-of-bounds handling
State: closed

We only have one test that goes through the OOB handling code, and that re-raises within the OOB handling.  We're better off being strict here like everywhere else rather than having spotty support.

Labels:
name: Datetime
description: Datetime data dtype
name: Frequency
description: DateOffsets
name: Clean

Comments:
clicking through to travis shows green, just hasnt propagated here
thanks!
DOC: Add plotting examples and fix broken examples
State: closed

This PR improves the `pandas.plotting._misc.py` module docs by:
1. Adding example plots where they were missing from docstrings.
2. Fixing examples that were not formatted to render with
```
.. plot::
    :context: close-figs
```
3. Simplifying the syntax of some oddly-written examples.

In some cases, I replicated the examples found in the [User Guide](https://pandas.pydata.org/pandas-docs/stable/user_guide/visualization.html#plotting-tools).

Labels:
name: Docs
name: Visualization
description: plotting

Comments:
Hello @deppen8! 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-05-05 21:05:27 UTC
Thanks @deppen8, cool stuff. Did you render the website to see that everything is looking as expected?

@jorisvandenbossche, may be you want to have a look here.
@datapythonista, I built the docs locally and nothing looked out of order.

@jorisvandenbossche I found this advice which suggested using random seeds.

https://pandas.pydata.org/docs/development/contributing_docstring.html#tips-for-getting-your-examples-pass-the-doctests

In the case of the lag plot, it has an added benefit of keeping the demo data above zero, which probably matches most use cases better.
> I found this advice which suggested using random seeds.

Ah, yes, but that's only to get doctests passing where eg a number is shown in the output, while here the plots are not actually checked.

For the lag plot that sounds like a good reason, for the others I would maybe remove it again, if we don't do that elsewhere
> For the lag plot that sounds like a good reason, for the others I would maybe remove it again, if we don't do that elsewhere

Done!
Thanks @deppen8 !
BUG: fix memory_usage method with deep of StringArray
State: closed

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


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

Comments:
@kotamatsuoka thanks a lot for working on this!
thanks @kotamatsuoka 

as a followup can you add tests for each EA that we have. note that it *is* possible for memory usage under deep=True to be equal to deep=False (for some implementations).
BUG: Fix Series.update ExtensionArray issue
State: closed

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

Comments:
@jreback ping
thanks @dsaxton 
hmm, the tests added here fail when backporting this fix. There have been many refactors of the indexing code since 1.0.3
> hmm, the tests added here fail when backporting this fix. There have been many refactors of the indexing code since 1.0.3

removing the needs backport label for now.
@simonjayhawkins generally in backport i would do the least possible ones to avoid new issues
this PR did fix a regression https://github.com/pandas-dev/pandas/issues/34020#issuecomment-624591164

so was a candidate for backport. 
i understand but my point is that we only want backport a very minutes number of tegressions - esp later in a release series