BrandonFors/pandas-github-issues huggingface dataset:
Remove blocks from GroupBy Code
State: closed

Just trying to simplify things...

Currently fails (locally at least) with 5 tests, which interestingly enough seem mostly related to `mean`. Need to debug further but submitting now for feedback

Labels:
name: Groupby
name: Clean

Comments:
Is the idea to operate column-by-column instead of block-by-block?
> Is the idea to operate column-by-column instead of block-by-block?

I think that's what it will end up being. There are a lot of code paths in groupby that can be simplified and I *think* that iteration over elements is inconsistent. This is block iteration and we separately have iteration maybe by Series or Frame elements (depending on whether or not there are duplicate column labels) which makes the code harder to reason about and buggy

So not going strictly column by column in this PR yet; just removing what I think is a one-off code path to make things easier and will leave more for follow ups
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 01:29:39 UTC
Any idea of perf impact?  Presumably wide-block cases will be the hardest hit right?
> Any idea of perf impact? Presumably wide-block cases will be the hardest hit right?

I think generally so yes, but unfortunately will depend on function (some don't go through the block management code). Once I get green in CI will run some tests and maybe add a few if coverage is bare, so will have something more tangible for you then
This was a good exercise but I think need a few pre-cursors before coming back to this. Depending on how hard it is to rebase may ultimately end up with a new PR, but will see
> This was a good exercise but I think need a few pre-cursors before coming back to this.

Definitely +1 on the concept.  Pending the performance hit, I'm looking forward to seeing this make a comeback.
The more I work in/around this method, the more I think it needs to DIAF
DOC: fix DataFrame.plot docstrings errors PR09,PR08
State: closed

this is related to #28702
also fix a few formatting issues
related to default arguments
and argument ordering

- [x] tests added / passed
- [x] passes `black pandas`

Labels:
name: Docs

Comments:
@josibake do you mind merging master and fixing the conflicts. This is ready to be merged when you do. Thanks!
@josibake looks about ready, can you rebase
@josibake pls rebase
Closing as stale.  Feel free to re-open if you want to continue here.
TST: port more maybe_promote tests, adds 260 xfails
State: closed

Currently working on a branch that fixes (some of) these new xfails.

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
not really happy about adding these; but hopefully should simplify after you have all tests in and can rip out things.
TST: Remove maybe_promote tests for iNaT as an NA value
State: closed

These are left over from a time when maybe_promote treated iNaT as an NA value, which we stopped doing because it is ambiguous.

Removes 22 xfails (leaving 317 in this file)

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
cool
PERF: block-wise ops for scalar and series
State: closed

This extends #28583 to handle `op(frame, series)` (whereas 28583 handles only `op(frame, scalar)`).

I got this passing locally, haven't gone back through to do e.g. lint fixups.  There are a few places where this surfaced bugs that can be fixed independently.



Labels:

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-14 16:34:09 UTC
Closing to clear the queue.
hide Index.get_values in docs and IPython tab completion
State: closed

In #28621 I hid the deprecated method ``Index.set_value`` in the docs (in ``indexing.rst``) and in the REPL/Ipython (by adding it to ``Index._deprecations``). I like that as it hides not-recommended parts of the pandas API and guides users away from deprecated methods.

I've given the already deprecated method ``Indexc.get_values`` the same treatment in this PR. If there's consensus about it, I could go through various deprecations (at least the more obscure ones, like ``get_ftype_counts`` and ``get_ftype_counts`` etc.).

Labels:
name: Docs
name: Deprecate
description: Functionality to remove in pandas

Comments:
Does this make the doc unaccessible or just not shown in the API reference? 
The removes the method name from Ipython (``Index._deprecations``) and in the doc TOC (``indexing.rst``). The underlying document is however still around, e.g. for ``Index.set_value``, see here: https://dev.pandas.io/docs/reference/api/pandas.Index.set_value.html#pandas.Index.set_value

My interest is getting these out of the ipython tab-completion, so tabbing is more focused. I could revert the removal in the doc TOC, as I can see it's reasonable that deprecated methods are shown there.
Makes sense. I'm fine with this either way - @TomAugspurger or @jorisvandenbossche might have some insights into how we've historically handled deprecated functions in the TOC
+1 on removing them from tab completion (did that recently as well for attributes).

For the docs, in the autosummary tables they get a clear "DEPRECATED" in front (see eg in https://pandas.pydata.org/pandas-docs/stable/reference/series.html), there has been done some work to do that automatically. So I am also fine with keeping them, as it is quite visually clear they are deprecated. But of course, if it are many, it also clutters the page. 
(and for people looking at those pages, they might actually realize that some methods they are using are deprecated) 
But no strong opinion about it.
thanks . I wonder if we can have a test generally to avoid these issues? e.g. we should try to see if any tab completables show a deprecation warning and then fail the test.
@jreback I answered about that in the other PR as well: https://github.com/pandas-dev/pandas/pull/28524#issuecomment-533609025 
Someone would need to find out how to write a test directly with jedi, as we already have this test for IPython and it doesn't catch those warnings. 
Also, this is only for attribtues. Methods are not executed so don't raise a warning on tab-completion, so can't be catched that way.
TST: port maybe_promote tests from #23982
State: closed

Not all of them, but whittling them down.

The tests are copied almost verbatim.  The only change is updating the datetime64/timedelta64 expected results to be datetime64/timedelta64 instead of iNaT.

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
how many xfails does this add?
135.  That'll get whittled down pretty quickly in follow-ups, but I don't want to mix-and-match porting these with fixing them
k thanks
CLN: Define UDFException for catching arbitrary Exception
State: closed

Alias `UDFException = Exception` (User Defined Function) for cases where we can't be more specific than `except Exception`, but this way we can be explicit that we're doing this on purpose.

Maybe at the end of this process we can add a code check to disallow `except Exception` altogether.

Labels:

Comments:
I would personally rather just use `except Exception` for this, as this is plain python that is working fine. 
(in any case, if we add it, it should be internal, and not in the public `pandas.errors` IMO)

If it is for linting, we can also require a specific:

```
except Exception:  # noqa
```

and only have a code check for `except Exception:` without that additional `# noqa`. This way, we also have a way to be explicit about doing a `except Exception` on purpose.
agree with @jorisvandenbossche here; we shouldn’t have an  exception catchall that is custom
Fair enough.  Closing.
BUG: Allow cast from cat to extension dtype
State: closed

- [x] closes #28668
- [x] tests added / passed
- [x] passes `black pandas`
- [x] whatsnew entry

Modifies `categorical.astype` to allow for casting to extension dtypes.  Also fixes the merge issue identified in #28668.

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

Comments:
Getting this `mypy` error: `pandas/core/arrays/categorical.py:527: error: Argument 1 to "array" has incompatible type "Categorical"; expected "Sequence[object]"`.

Should `pandas.core.construction.array` be able to accept type `Categorical`?
Hmm have we ever considered making the `ExtensionArray` definition align with any of the Python abcs? To align with the `Sequence` interface would need `__reversed__`, `__contains__`, `index`, and `count`. `Collection` would only require `__contains__` out of those

@TomAugspurger any thoughts? Would certainly help the type system
I thought it was an instance of abc.Sequence, but apparently not. Apparently, ndarray's aren't Sequences either...

I don't have a strong opinion here.
I opened #28770 as a follow up to that. @dsaxton for now I think you can just `# type: ignore` that and ref the open issue
thanks @dsaxton
DOC: Fixed PR06 docstring errors in pandas.interval_range & pandas.util.hash_array
State: closed

DOC: Fixed PR06 docstring errors in pandas.interval_range

- [ ] xref #28724 
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Docs

Comments:
lgtm. @datapythonista 
Thanks for the fixes @samyak-jn
Changed pypi tag of Cython
State: closed

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


Labels:
name: Clean

Comments:
both versions work, why should we prefer the uppercase one?
Feels like hacktoberfest stuff, closing.
TST: Fix maybe_promote xfails
State: closed

Fix a handful of these, clean up a bunch of others.


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

Comments:
This gets towards the end of what I can clean up without major changes.  I'm inclined to try to remove the `box=True` cases and disallow ndarray `fill_value` arg, which would simplify the code a bit and the tests a ton.  AFAICT we can do this without breaking any existing tests (outside of tests for `dtypes.cast`), but I don't think we can rule out it being _possible_ to get there in the handful of places where maybe_promote is called.
Reverted unnecessary test cleanup to make a smaller diff
maybe rebase this as we merged a few other things, ping on green.
sure.  heads up: im out of town until monday, will be less responsive than usual
ping
thanks
Eliminated _WriterBase class, removed unused fixtures from methods in pandas/io/excel/test_writers.py
State: closed

- xref #26784
- passes `black pandas`
- passes `git diff upstream/master -u -- "*.py" | flake8 --diff`


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: IO Excel
description: read_excel, to_excel
name: Clean

Comments:
Hello @dayoreke! 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-13 21:31:37 UTC
> Thanks for taking a look - this one might be tricky.

this addresses Eliminate _WriterBase class and engine and ext injected into tests https://github.com/pandas-dev/pandas/pull/26662#discussion_r291260131 from #26784

could split into two PRs if it makes it easier to review.
test_basics_with_nan failing.
Thanks @dayoreke Very nice!
DEPR: is_copy arg of take closes #27357
State: closed

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


Labels:
name: Deprecate
description: Functionality to remove in pandas

Comments:
@chrisstpierre is this still active? If so can you merge master and repush?
Closing as I think this is stale but @chrisstpierre ping if you'd like to pick back up
get rid of the None engine tests and just have a separate test that ensures how that gets bound to a particular engine #26662 (comment)
State: closed

- [x] xref https://github.com/pandas-dev/pandas/pull/26662#discussion_r291258182, https://github.com/pandas-dev/pandas/pull/26543#issuecomment-497126487, #26784
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] I've replaced `None` engine tests with corresponding separate test for default engine value. I also excluded `test_bad_engine_raises` from `TestReaders` class.


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: IO Excel
description: read_excel, to_excel
name: Clean

Comments:
Thanks for the PR. I think this is OK as is in master though, so let's table this for now
Test pytables refactor
State: closed

 - precursor to #18498 and continuation of #28715
- No new tests added
- black pandas ran successfully
- This PR is the next step reference in #28715. 

@simonjayhawkins and @jreback as promised, this PR contains the following new and EXCITING changes:

1. Created a contest.py in the pytables sub directory containing all the shared fixtures
2. Created common.py which contains all the functions and variables used by the test classes such as tables, safe_remove, safe_close, create_tempfile, ensure_clean_path, ensure_clean_store, _maybe_remove
3. Created separate modules for each of the test classes i.e. test_timezones, test_hdf_store, test_hdf_complex_values
4. As a result of creating the common.py, I have update the ensure_clean_path import to the test_compat module to ensure safe deletion of the test_pytables module.

There has been no amendments to the body of the code. The next PR will involve focusing on removing the class in test_timezone and making it strictly functional. Followed by me doing this to the other 2 new modules i.e. test_hdf_store and test_hdf_complex_values.

Once the code base is strictly functional, I can start working on more fun refactoring of the functions :)

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: IO HDF5
description: read_hdf, HDFStore
name: Clean

Comments:
Hello @tolaa001! 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-04 10:09:29 UTC
@tolaa001 you have some failing checks, if you can fixup we should merge this soon to prevent conflicts.
> @tolaa001 you have some failing checks, if you can fixup we should merge this soon to prevent conflicts.

@jreback all green now and reverted my accidental changes to test_operators.py. Thank you again for reviewing this
thanks @tolaa001 very nice.

love to have the followups as discussed above.
DOC: Styler errors PR08 and PR09
State: closed

-  closes #28656
-  tests added/ passed - Checked with 
```
./scripts/validate_docstrings.py pandas.io.formats.style.Styler --errors=PR08,PR09
```
- passes `black pandas`
- passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- whatsnew entry - not needed


Labels:
name: Docs

Comments:
@bhuvanakundumani Thanks a lot !
> @bhuvanakundumani Thanks a lot !

Thanks @jorisvandenbossche . 
Thanks
DOC: Pandas.Series.drop docstring PR02 (#27976)
State: closed

- [x] addresses, but **does not fully close**, #27976 
- [x] tests added / passed - I didn't run the full `pytest` suite on the actual code, but rather re-ran:
```
./scripts/validate_docstrings.py pandas.Series.drop --errors=PR02
```
to confirm that the docstring formatting passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] ~whatsnew entry~ - not needed

First PR to pandas, which has been my go-to tool for many years now! Thanks, maintainers! 🎉 


Labels:
name: Docs

Comments:
Thanks @angelaambroz !
BUG: Fix DataFrame logical ops Series inconsistency
State: closed

closes #5284 
closes #5035
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Will need to look and see if this closes anything else.

This will have a merge conflict with #28638, that is a slightly higher priority.

Not sure if this constitutes an API change as the behavior for these ops is all over the place.



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

Comments:
pls rebase
rebased+green
lgtm. I think we need a small section in whatsnew to show what has changed? (could defer if you want to do a larger section on ops changes, though maybe easier to grok in pieces).
Added line in whatsnew, not a whole section.  Prefer to hold off on more pending nailing down remaining inconsistencies
thanks. I would add the original issues to the whatsnew that you have here (and maybe in the comments you added in ops.py)
DOC: fix PR09,PR08 errors for pandas.Timestamp
State: closed

Alright, trying this again. 

Note: for `Timestamp.tz_localize` , the `deprecated:: 0.24.0` was in the wrong place. the indentation implied it was only deprecating the `coerce` argument of the `errors` parameter, but if you read both of the arguments for `errors`, it implies the entire parameter is deprecated in favor of `nonexistent` (added in 0.24.0)

Regarding the length of this PR, it also involved editing the NaT type class and Timedelta class. This is because there is a validation which enforces all of these classes to have the same docstrings.

- [x] closes #28673
- [x] tests added / passed

Labels:
name: Docs

Comments:
lgtm as well, though maybe @jbrockmendel can confirm intent on deprecated argument
Couple of nitpicks, all of which can be follow-ups if preferred.  Otherwise LGTM.
@jbrockmendel a few open questions in my responses 
I think the ValueError edit needs tot be reverted and we need an opinion from datapythonista on a long set of literals.  Once those two are addressed, LGTM.
Thanks @josibake
Backport PR #28400 on branch 0.25.x (Fix a typo in "computation.rst" in document.)
State: closed

Manually backporting #28400 since the bot isn't responding; did a cherry-pick of 6d47fabaf94d7427b48b30a3808545738cd778c6.  This should fix the failing doc builds on the 0.25.x branch.


Labels:
name: Docs

Comments:
Thanks @jschendel
Issue 28518 multiindex interesection
State: closed

MultiIndex lexsort_depth now return sortorder if sortorder is not set to None

At first the error come from the intersection of the two indexes, that resulted in an unsorted index with an invalid lexsort_depth value. Then when looking at the lexsort_depth function I found the deeper error, where the returned lexsort_depth value were the number of level instead of the self.sortorder value.

After fixing that I implemented a check in the MultiIndex._verify_integrity function when a sortorder is given to raise if it no coherent with the lexsort_depth.

I added tests for the issue, for the lexsort_depth function, and fo raising error in the _verify_integrity function.

- [x] closes #28518
- [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
name: MultiIndex

Comments:
@jreback Changes are made and checks are green. 🙂
@jreback Green and ready for takeoff
thanks @nrebena very nice!
BUG: fix renaming in requirements generation script
State: closed

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


Labels:
name: Dependencies
description: Required and optional dependencies

Comments:
cc @TomAugspurger 
Thanks!
Inconsistent indexes for tick label plotting
State: closed

The tick position for BarPlot can be define using the convert tool from matplotlib.

The main advantage is that it will reuse the same position when you have text as axis values. Previously, the tick position was determined by the order of the given element, so that ['A', 'B'] where given label [0, 1], and if updating the plot with order ['B', 'A'], you will not draw at the right position.
### 
The resulting plot for each issue would be:
![issue26186](https://user-images.githubusercontent.com/49879400/65983815-b24e5100-e47e-11e9-87c5-8c536744da28.png)
![issue11465](https://user-images.githubusercontent.com/49879400/65983825-b8443200-e47e-11e9-954d-76919e96c38f.png)
- [x] closes #26186
- [x] closes #11465
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Visualization
description: plotting

Comments:
So one of the example of the doc revealed a bug in this PR. 
I added it to the test suite and hopefully will fix it. It is about using a MultiIndex of string (or containing a string in a level?) as axis label.
This PR solved the bug related to index, but handling MultiIndex in the same way requires more work and I consider it outside the scope of this PR, so I expressely did not handle it.

The best way to handle MultiIndex plot would be via label grouping and multi-level axis label, as described in this issue on matplotlib matplotlib/matplotlib#6321, and this one on pandas #2088. I may try to tackle this one day, but definitly not in this PR.

can you merge master and will look again
I made the dtype for tick array as int as pointed by @WillAyd, see last commit change.
I also rebased on master.
@nrebena I think this should be good. Can you fix the merge conflict?
Rebased and green :green_apple:  @WillAyd 
Hi @nrebena - sorry to chase you up, just wanted to ask if this is still active
Hi. No worries. I may come around to finish it, but if someone want to take it over no problem. 
Rebased on master and correction done.
@jreback I improve the warning behavior to warn about REdrawing a bar plot with a MultiIndex, and add comment in the test to make it more explicit what is does.
can u merge master and resolve issues and will look
@nrebena can you move release note to 1.2 and merge upstream/master to resolve conflict
> @nrebena can you move release note to 1.2 and merge upstream/master to resolve conflict

Hi @nrebena - could you address Simon's comment if this is still active? It looks like it's pretty close to being ready
@nrebena can you merge master once more?

ping @jreback 
thanks @nrebena , very nice PR!

thanks @MarcoGorelli for resolving the conflicts!
this PR was reverted, but I guess we don't have a tracking issue. @nrebena can you open one.