BrandonFors/pandas-github-issues huggingface dataset:
TST: GH30999 TESTING PR do not merge or review
State: closed

There are 3 tests that don't run on my dev machine even with tweaks. They should fail in the CI and I can get the messages from there. So for now this is just a PR designed to run (and fail) the CI pipelines.

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


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

Comments:
Well. The CI succeeded without errors. Did not work as I intended.
When you run pytest, you should see a message to this effect:

> test_sql.py::TestXMySQL::test_execute_fail SKIPPED                       [100%]
Skipped: gh-13611: there is no support for MySQL if SQLAlchemy is not installed

which is curious because I know I have SQLAlchemy installed. So a quick search for "13611" in the test code reveals:

    @pytest.mark.skip(
        reason="gh-13611: there is no support for MySQL if SQLAlchemy is not installed"
    )
    class TestXMySQL(MySQLMixIn):

Looks like this entire class is skipped even when SQLAlchemy is installed (there is no check). Uncommenting that line these tests need `pymysql` installed to run; and even with it installed they all fail for me.
@rhshadrach Yes they fail for me too. That's why I decided to try in the CI pipeline.

I don't know where to go from here. I guess the options are:
1. Leave as-is, and leave the directories tests/compat and tests/io excluded from the pylint rule to be added as part of #30999 
2. Delete these tests entirely, since they seem to not ever be running anyway
3. Spend a good chunk of time really figuring out what is going on here, and what is the intended behaviour, including probably having to correctly configure MySQL on my dev machine
> That's why I decided to try in the CI pipeline.

Don't you need to remove the `pytest.mark.skip` to try it?
> > That's why I decided to try in the CI pipeline.
> 
> Don't you need to remove the `pytest.mark.skip` to try it?

Oh right. Thanks. That was a bit dumb of me.
Setting up the pytest fixture doesn't work in the CI, just like on my dev machine. It looks like #13611 is open to remove most of these tests. I don't know if we want to bring that higher in the priority - I don't feel it's something I am qualified to do.
TST: Series.update with categorical
State: closed

- [ ] closes #25744, closes #27338
- [ ] 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
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Categorical
description: Categorical Data Type

Comments:
Hello @ftrihardjo! 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 2021-01-03 02:14:22 UTC
Hi @ftrihardjo - could you please put a meaningful title and link to the issue you're closing in the pull request body?
see here: https://github.com/pandas-dev/pandas/pull/38873/checks?check_run_id=1634842619

make sure you have `precommit` installed it will fix anything when you commit
thanks @ftrihardjo
REF: Move aggregation into apply
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

This is a step toward #35725. We can't simply ban agg from not aggregating because it's used by apply for series, dataframe, groupby, resampler, and rolling. There are two other related issues. First, apply, agg, and transform are all doing similar things but with different implementations leading to unintended inconsistencies. Second, we would also like to ban mutation in apply (and presumably agg and transform for the same reasons).

It seems best to me to handle all of these by first combining the implementation into apply, cleaning up and sharing code between the implementations, and then we would be in a good place to ban agg from not aggregating.

This starts the process by moving aggregation.aggregate into apply, but it is only utilized for frames. Once this is done for the remaining objects (series, groupby, resampler, rolling), the rest of the functions in aggregation can be moved into apply as well.

Labels:
name: Refactor
description: Internal refactoring of code
name: Apply
description: Apply, Aggregate, Transform, Map

Comments:
Thanks @jreback, changes made and green.
ENH: implement FloatingArray.round()
State: closed

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

Labels:
name: Series
description: Series data structure
name: NA - MaskedArrays
description: Related to pd.NA and nullable extension arrays
name: Stale

Comments:
milestoned as 1.2.1. ok to merge changes to experimental types in patch releases?
these generally should not be backported as EAs (esp) Floating are still experimental
> milestoned as 1.2.1. ok to merge changes to experimental types in patch releases?

we can do on a case by case. the proposed fix here that i suggest is a bit more non-trivial.
Since I think being able to round is a quite essential functionality for a floating dtypes that we missed, I think it makes sense to backport this, exactly because it is still experimental anyway. 
(since it's experimental anyway, we don't need to be as careful to backport as for other parts of pandas, and by backporting this, it will make it easier for users to actually already experiment with the new dtype in pandas 1.2.x, and thus we can get more feedback)
> Since I think being able to round is a quite essential functionality for a floating dtypes that we missed, I think it makes sense to backport this, exactly because it is still experimental anyway.
> (since it's experimental anyway, we don't need to be as careful to backport as for other parts of pandas, and by backporting this, it will make it easier for users to actually already experiment with the new dtype in pandas 1.2.x, and thus we can get more feedback)

I don't think we should be backporting things like this.
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.
closing as stale. I think we need the general soln outlined above. Happy to re-open / take new PR.
TST: GH30999 add match=msg to 2 pytest.raises in pandas/tests/io/json/test_normalize.py
State: closed

xref #30999 for pandas/tests/io/json/test_normalize.py

And with this my work on #30999 is done. Will head over to the issue to explain why I am not fixing the remaining 15 instances of `pytest.raise`

- [ ] 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:
@github-actions precommit
actually its the ci/checks. @moink pls fixup when you can
Hmm I believe the check is saying that one test is using` pytest.raises` but not as a context manager. That's not actually true - I think it's a false positive, it's just a slightly weird structure of passing in a context manager as an argument.

I will see about restructuring the test so it doesn't give that error anymore. Is it a new check? Because it's not a new structure.
thanks @moink
BUG: additional keys in groupby indices when NAs are present
State: closed

- [x] closes #9304
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

`lib.indices_fast` included logic to skip over null values (-1's), but did not include the case where the first index is null. Currently this function is only used in `sorting.get_indexer_dict` where the case of all null group_index is handled separately.

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

Comments:
@jreback thanks for the review; responses above.
thanks @rhshadrach
DOC/CI: Fix building docs with --no-api
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Fixes running docs via

     python make.py --no-api

A few notes:

 - The removed check `if os.path.exists(path):`; will incorrectly raise if docs are being rebuilt.
 - The value `dirname` returned by `os.walk` is an absolute path, but the check was treating it as relative.
 - If docs are built with api and then rebuilt with --no-api, we want to exclude everything in the `reference` directory; in particular `reference/api`.

Labels:
name: Docs
name: CI
description: Continuous Integration

Comments:
@github-actions pre-commit
lgtm.
BUG: DataFrame(ndarray, dtype=categoricaldtype)
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: Categorical
description: Categorical Data Type
name: Constructors
description: Series/DataFrame/Index/pd.array Constructors

Comments:
this broke some things locally, am troubleshooting now
rebased + green
TST: GH30999 address all bare pytest.raises in pandas/tests/series and add EMPTY_STRING_PATTERN to tm
State: closed

This PR is to address xref #30999 in pandas/tests/series

Three of the four errors raised within `pytest.raises` were for `NotImplementedError` with the empty string as the error message. Rather than individually making a regular expression for the empty string, I added one as a constant to `pandas/_testing/__init__.py` . There are two more similar cases in other modules so I wanted to put it somewhere that all the testing modules import. But if you don't like it I can revert it and instead use an individual empty string pattern in each case.

- [ ] 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:
thanks
Fix broken link in docs of DataFrame.to_hdf
State: closed

The link in https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.DataFrame.to_hdf.html in "See also" seems to be broken. This should fix it.

- [ ] 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:
thanks @felixdivo
CI,STYLE: narrow down ignore-words-list of codespell
State: closed

- [x] xref #38820 
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry  
see Issue #38802, task4

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

Comments:
That's a good point, thanks @jorisvandenbossche 

So, do you suggest to keep this PR's renaming of `datas -> data`, but put `fo` and `ba` back into `ignore-words-list`, remove `.codespellignorelines`, and remove the `exclude-file` option in `setup.cfg`?
@MarcoGorelli I agree with @jorisvandenbossche to remove the `.codespellignorelines` file and bring `ba` and `fo` back into the `ignore-words-list`. Otherwise it may be convoluted and brittle as @jorisvandenbossche pointed out.
@MarcoGorelli I fetch from `upstream/master` before I commit my modification, so the commits others have done are merged. Do I need to fetch from the upstream before I commit or the PR system will deal with the my version which have fallen behind the upstream to be merged correctly?
@chinggg it's fine to fetch before adding new commits, but that shouldn't show so many changed files - without seeing the exact commands you ran, it's hard to guess what may have gone wrong

I think now I'd suggest
```
git rebase upstream/master
git push -f
```
, when trying that locally from your branch it works fine
thanks @chinggg
Update conf.py to execute imports during pdf building
State: closed

closes #38451

According [https://ipython.readthedocs.io/en/stable/sphinxext.html](), it should be `ipython_execlines` not `ipython_exec_lines`. Should close issue #38451



Labels:
name: Docs

Comments:
hmm so it is.
cc @simonjayhawkins 
@simonjayhawkins how's this looking
will merge this to close #38451 as it does prevent the NameError.

However, something else is awry since `{{ header }}` still appears in the pdf docs.

During the first run of `./make.py latex_forced` it appears in 10 minutes to pandas and no contents are created. we run a second time (I assume to get the contents to build) and on the second run the visualisaztion section is recreated (why this happens is they key to solving this properly) but without the header code being run but the change here means that we don't get the NameErrors
Thanks @gepcel 
> However, something else is awry since {{ header }} still appears in the pdf docs.

That might because the section (reference) name is "visualization" and the section title is also "visualization", therefore the `Duplicate implicit target name: "visualization"`.
DOC: fix includes
State: closed

- Added missing `.` to the `.. include`s
- Put the includes in a folder to exclude from Sphinx, to avoid `:orphan:` being written to the HTML

Before:

<img width="1156" alt="Screen_Shot_2020-12-30_at_7_08_52_PM" src="https://user-images.githubusercontent.com/86842/103387595-99de4380-4ad2-11eb-85bf-8ac8da85676a.png">

After:

<img width="1186" alt="Screen Shot 2020-12-30 at 7 07 41 PM" src="https://user-images.githubusercontent.com/86842/103387540-5f74a680-4ad2-11eb-992c-44de074462ff.png">

---

- [ ] ~~closes #xxxx~~
- [ ] tests added / passed
- [ ] ~~passes `black pandas`~~
- [ ] ~~passes `git diff upstream/master -u -- "*.py" | flake8 --diff`~~
- [ ] ~~whatsnew entry~~


Labels:
name: Docs

Comments:
small linting error but the doc build passed: https://github.com/pandas-dev/pandas/pull/38839/checks?check_run_id=1628340684
thanks @afeld (pls check dev docs for rendered results in a while)
They look good!

- https://pandas.pydata.org/pandas-docs/dev/getting_started/intro_tutorials/03_subset_data.html
- https://pandas.pydata.org/pandas-docs/dev/getting_started/intro_tutorials/05_add_columns.html
DOC: create shared includes for comparison docs, take II
State: closed

This is a duplicate of https://github.com/pandas-dev/pandas/pull/38771, but instead of defining `:tips:` in a `:suppress:` block, it attempts to tell flake8-rst to ignore those warnings. In other words, only one of these pull requests should be merged; the other can be closed.

- [ ] ~~closes #xxxx~~
- [ ] tests added / passed
- [ ] ~~passes `black pandas`~~
- [ ] ~~passes `git diff upstream/master -u -- "*.py" | flake8 --diff`~~
- [ ] ~~whatsnew entry~~


Labels:
name: Docs

Comments:
great. let's see how the ci does.
As [expected](https://github.com/pandas-dev/pandas/pull/38771#issuecomment-751955518), it's getting `F821 undefined name 'tips'` in all those include files that reference that variable. Very possible I just don't know what I'm doing and missed some part of the configuration.
The doc build is failing because flake8-rst is not installed in that env:

```
Could not import extension flake8_rst.sphinxext.custom_roles (exception: No module named 'flake8_rst')
```

Now, for the pre-commit check, that indeed fails. 
For me locally, I get the same failure. If I do `:flake8-group: Ignore` (which ignores the block completely), then it doesn't complain. 

Short term, I would maybe just add those `*/includes/*` files to the exclude list of flake8-rst:

https://github.com/pandas-dev/pandas/blob/72ad7178cc584e2048bc26e773249fb9579e6ca0/setup.cfg#L51-L52

And then I would open an issue at https://github.com/kataev/flake8-rst reporting that the `:flake8-add-ignore: F821` is not working for this example (it might be a bug in the package).

> I would maybe just add those */includes/* files to the exclude list of flake8-rst

Done: https://github.com/pandas-dev/pandas/pull/38887 (Sorry for the litany of pull requests; just wanted to make sure the different approaches can be seen independently.)

> I would open an issue at https://github.com/kataev/flake8-rst reporting that the `:flake8-add-ignore: F821` is not working for this example

Done: https://github.com/kataev/flake8-rst/issues/25
#38887 is before this? should this close?
Closing in favor of https://github.com/pandas-dev/pandas/pull/38887.
BUG: IntervalIndex.intersection returning duplicates
State: closed

- [x] closes #38743
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Moved the wrong test code into a new test with adjusted behavior


Labels:
name: Interval
description: Interval data type
name: Index
description: Related to the Index class or subclasses

Comments:
can you also add the exhaustive test from the op

```
def check_intersection_commutative(left, right):
    assert left.intersection(right).equals(right.intersection(left))

@pytest.mark.parametrize("index_maker", tm.index_subclass_makers_generator())
def test_intersection_uniqueness(index_maker):
    if index_make is tm.makeMultiIndex:
        pytest.pass()  # This index maker is bugged
    ind_unique = index_maker(k=5)
    ind_non_unique = idx_unique[[0, 0, 1, 2, 3, 4]]

    check_intersection_commutative(ind_unique, ind_non_unique)
    assert idx_unique.intersection(idx_non_unique).is_unique
```

except use the index fixture
Done, moved it into the regular setops file.
thanks @phofl
Parametrized groupby allowlist test
State: closed

Still exploring a few things to remove setting the .data pointer in Cython - this test is liable to fail but very tough to debug with current loop

@jbrockmendel 

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

Comments:
LGTM pending green

https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=51316&view=logs&j=eab14f69-13b6-5db7-daeb-7b778629410b&t=ce687173-08c6-5301-838d-71b2dda24510 is an actual failure (tshift is deprecated)
thanks @WillAyd
CLN: add typing to dtype arg in selection of files in core/arrays (GH38808)
State: closed

Follow on PR for #38808
- [ ] 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: Typing
description: type annotations, mypy/pyright type checking

Comments:
Some checks are failing, will look at it tomorrow.
lgtm ping on green
thanks @avinashpancham
TST: GH30999 Change all pytest.raises in pandas/tests/indexing to tm.external_error_raised
State: closed

Addresses xref #30999 for pandas/tests/indexing by changing three bare `pytest.raise` instances to `tm.external_error_raised`

This is the last of the simpler PRs for #30999 - after that it gets more complex.

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


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

Comments:
thanks @moink
TST:  add missing loc label indexing test
State: closed

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

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

* In the file [`pandas/tests/indexing/test_loc.py`](https://github.com/pandas-dev/pandas/blob/0a25025f37ef4e8ae8c60c4e4ca2db07ef1bc83d/pandas/tests/indexing/test_loc.py):

> [Line 66:1](https://github.com/pandas-dev/pandas/blob/0a25025f37ef4e8ae8c60c4e4ca2db07ef1bc83d/pandas/tests/indexing/test_loc.py#L66): [W293](https://duckduckgo.com/?q=pep8%20W293) blank line contains whitespace


sure, there are some linting issues
CI,STYLE: GH38802 narrow down list of ignored words
State: closed

- [x] closes #38811 
- [ ] 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:
Thanks a lot for explaining patiently!  
I hava realized the general working mechanism of `codespell`. That is very cool and must be useful in large projects like pandas. We should consider how frequently and commonly the words are used by programmers around the world, and only add those widely-accpeted acronyms or abbreviations to the `ignore-words-list`. Those temporary symbols should be put in the `.codespellignorelines` file instead.
Because I do not live in an english-speaking country, I am not sure about the frequency of these shorten words. So I use `codespell` to check where and how they show up in the code.  
I found that
- `blocs` may be a common abbr. for `blocks`  
- `coo` seems to be a mathematical term  
- `nd` stands for N-dimensional  
- `ser` is short for `series`
- `hist` only has 4 results but it is also used by `matplotlib` so I consider it as a common practice.  
So I decide to remove only `ba` and `datas`(already done by your instruction to rename `datas` to `data`).  
I make a new branch named `codespell-ignore`. Because the branch of this PR is `master` and seems to be unchangeable, so I have to make a new PR again. I hope it will not be improper.
Sure, that's fine, thanks!

And yes, I agree with your assessment of which words to keep in `ignore-words-list`
>  I have to make a new PR again. I hope it will not be improper.

Closing this one in favour of the new PR then
REGR: read_excel does not work for most file handles
State: closed

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


Labels:
name: Regression
description: Functionality that used to work in a prior pandas version
name: IO Excel
description: read_excel, to_excel

Comments:
@jreback  green
thanks @twoertwein very nice
@meeseeksdev backport 1.2.x
TST/CLN: Remove duplicate abc tests
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

From #38588, this test is a proper subset of the tests immediately below them.

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

Comments:
thanks @rhshadrach
MAINT: regex char class improve
State: closed

* remove superfluous regex character classes
from the codebase (those that contain a single
character incur the overhead of a class with
none of the advantages of a class)

* for more details, see similar change in NumPy:
https://github.com/numpy/numpy/pull/18083

* check performed with some simple [scraping code](https://github.com/tylerjereddy/regex-improve)

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

Comments:
thanks @tylerjereddy