BrandonFors/pandas-github-issues huggingface dataset:
TST/REF: consolidate hist tests
State: closed

- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Splitting up #38577, following @ivanovmg's comment. First step moves some `test_hist...` tests from `test_series.py` to `test_hist_method.py`, after this the remainder of `hist` tests in `test_series.py` will be duplicates to be deleted in a followup pr.

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

Comments:
great ping on green.
Mostly green, failures should be unrelated
thanks @mzeitlin11
BUG: remove unused tz keyword from PeriodIndex constructor
State: closed

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


Labels:
name: Period
description: Period data type

Comments:
has this been rebased after the deprecation PR? 
thanks
BUG: disallow scalar in CategoricalIndex construtor
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

Comments:
i guess need to deprecate here as well (unless you use the Categorical for constrution)
> i guess need to deprecate here as well

i think this is pretty unambiguously a bug
> > i guess need to deprecate here as well
> 
> i think this is pretty unambiguously a bug

oh right yeah its a scalar in an Index, ok
ok i guess add a whatsnew here to document
whatsnew added + greenish
CI: move py38 slow to azure #38429
State: closed

xref #38429

We're not testing databases on py38 slow currently. Keep the issue open until we figure out what to do with its SQL tests.



Labels:
name: CI
description: Continuous Integration

Comments:
thanks @fangchenli very happy to push :-> (as we have this travis deadline )
note that we will likley backport these to 1.2.x
@simonjayhawkins marking for 1.2 (but will just bump to actual 1.2.x version)
@fangchenli this looks good. ready? ignoring other builds failing on travis.
thanks @fangchenli keep em coming!
@meeseeksdev backport 1.2.x
Revert "REF: use astype_nansafe in Index.astype"
State: closed

Reverts pandas-dev/pandas#38518

Labels:
name: Regression
description: Functionality that used to work in a prior pandas version

Comments:
may also close #38592, see https://github.com/pandas-dev/pandas/issues/38592#issuecomment-748691750
~this is for 1.3?~ nvm i see

@jbrockmendel ok here ?
Yep

On Mon, Dec 21, 2020 at 8:41 AM Jeff Reback <notifications@github.com>
wrote:

> @jbrockmendel <https://github.com/jbrockmendel> ok here ?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/38610#issuecomment-749068803>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AB5UM6FNLYVJMCPUOO66GQLSV53EDANCNFSM4VEHFXZA>
> .
>

> may also close #38592, see [#38592 (comment)](https://github.com/pandas-dev/pandas/issues/38592#issuecomment-748691750)

hmm, maybe not.

```
       before           after         ratio
     [d3c52e4a]       [15daa4c5]
     <master>         <revert-38518-ref-astype-index>
-        9.82±1ms       7.09±0.1ms     0.72  stat_ops.FrameOps.time_op('sem', 'Int64', 0)

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.
```
thanks @simonjayhawkins 

@jbrockmendel might want to rebase any PRs which touch this
TST: GH30999 Add match=msg to all "with pytest.raises" in pandas/tests/io/pytables/test_store.py
State: closed

This pull request xref #30999 to remove bare `pytest.raises`. It doesn't close that issue as I have only addressed one file: `pandas/tests/io/pytables/test_store.py`. In that file there were 80 instances of bare `pytest.raises`.

`test_append_to_multiple_dropna_false` is marked as failing because it isn't raising the `ValueError` that it should. Because of this, I didn't know what the message should be, and reading through the code didn't help me figure it out. I wrote a draft error message and added a `TODO` (line 3790) and I hope that's the best way to handle it.

In `test_multiple_open_close` there were several assertions that `ClosedFileError` is raised with the same error message, with one in the middle that asserts that an `AttributeError` is raised with a different error message. I moved the one for the `AttributeError` to the end of the list (line 4229) to organize the assertions a little better and make clear that the `msg` parameter is the same for all the `ClosedFileError`s.

I did not add a whatsnew entry since it only changes the tests. Let me know if I should add one (and I am a bit unclear on how, i.e. what version this would end up in).

- [ ] xref #30999
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Error Reporting
description: Incorrect or improved errors from pandas
name: IO HDF5
description: read_hdf, HDFStore

Comments:
I can't figure out what went wrong with the Travis CI build. Any hints from anyone would be appreciated.
> I can't figure out what went wrong with the Travis CI build. Any hints from anyone would be appreciated.

ignore it, giving us trouble.
@moink can you merge master and ping on green (or greenish if the travis build fails like that again).
OT: @moink love to split this file up: pandas/tests/io/pytables/test_store.py in a separate PR :->
thanks @moink very nice
> OT: @moink love to split this file up: pandas/tests/io/pytables/test_store.py in a separate PR :->

I agree - I noted that when I made these changes I pushed `test_store.py` over 5000 lines. But I would note that the module it is testing is over 5200 lines and in my opinion also needs to be split up, and then the test module perhaps split up in the same way as the code module.

I am not actually knowledgeable about PyTables and I suspect wouldn't be the best person to do this task. Would you like me to create a new issue?
you can create a new issue

but splitting the tests first is much easier just for visibility
REF: simplify CategoricalIndex.__new__
State: closed

Part of a larger goal of making `Index.__new__` share code with `Series.__init__` and `pd.array`

Labels:

Comments:
There have been some performance regressions related to categorical, see https://pandas.pydata.org/speed/pandas/#regressions?sort=1&dir=desc (at date "2020-12-22 22:06"). This commit is part of the indicated range https://github.com/pandas-dev/pandas/compare/ca52e396...75d02c73, but there are some other categorical-related PRs of you as well in the same period, so didn't check which PR would be the cause. 

> There have been some performance regressions related to categorical

Can you point out one or two that you have in mind?
DEPR: allowing subclass-specific keywords in pd.Index.__new__
State: closed

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


Labels:
name: Deprecate
description: Functionality to remove in pandas
name: Constructors
description: Series/DataFrame/Index/pd.array Constructors

Comments:
can you merge master once more. i realized that the period keyword removal was merged *before* this
somethings still failing in precommit
nice!
CLN: Remove _get_roll_func
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: Clean
name: Window
description: rolling, ewma, expanding

Comments:
LGTM pending green
thanks @mroeschke
CLN: Window _constructors definitions
State: closed

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

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

Comments:
thanks @mroeschke
test case for issue #38267
State: closed

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


Labels:

Comments:
Hi @ftrihardjo - you've committed 2000+ files, something likely went wrong - see the [contributing guide](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html) for how to contribute
closing this, pls submit a new PR
TST: Added tests for ABC classes
State: closed

- [ ] closes #xxxx
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

`test_abc_types` is a subset of the tests added and can be removed in a followup.

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
Indeed, thanks @jbrockmendel. Changes made.
thanks @rhshadrach
ENH: Raise ParserWarning when length of names does not match length of data
State: closed

- [x] closes #21768
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

@gfyoung 

Raising ParserWarning now. Could change to FutureWarning, if we would like to deprecate for 2.0

As long as we are only raising a ParserWarning I am inclined to raise for trailing commas too.

Labels:
name: IO CSV
description: read_csv, to_csv
name: Warnings
description: Warnings that appear or should be added to pandas

Comments:
can you merge master.

I think this might be too noisy (in the real world) to raise on a trailing command as this is a common thing to write for csv formats.
Merged master, so we should avoid raising the warning when one set of trailing commas is given?
> Merged master, so we should avoid raising the warning when one set of trailing commas is given?

yeah i think so
This should do the trick. One set of trailing commas is allowed
can you merge master.

cc @gfyoung 
Merged
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.
@jreback Open point here:

Should we raise always when having more separators than in the header column or is one more ok?
i think we might break things but can u show an example of what u mean
Ok to summarize: raise means warn

The following should definitely not raise:
```
stream = StringIO("""
one,two,three
foo,bar,baz
""")
pd.read_csv(stream, index_col=False)
```

Header lenght equals data length, so everything is fine.
The following should raise:
```
stream = StringIO("""
one,two,three
foo,bar,baz,bam
""")
pd.read_csv(stream, index_col=False)
```
The data length is greater than the header length, so we lose data through the reading process. 

The open point is, if the following should raise:

```
stream = StringIO("""
one,two,three
foo,bar,baz,
""")
pd.read_csv(stream, index_col=False)
```

One trailing comma, so you could argue, that no data loss occurs. @jreback mentioned above, that this case should not raise, which makes the detection a bit harder.
this looks reasonable.

any comments @pandas-dev/pandas-core 
@phofl can you rebase and some questions above
I think I have adressed all comments
@phofl can you resolve conflicts. 
resolved conflicts, @jreback ready to merge?
thanks @phofl 
@meeseeksdev backport 1.3.x
Something went wrong ... Please have  a look at my logs.
TYP: Added cast to ABC EA types
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 the last set of ABCs to cast. Going out with a whimper, mypy didn't identify any new issues.

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

Comments:
can you merge master and ping on green (as just merged the added tests for these), still expect it to work, but just in case.
thanks @rhshadrach
BUG: MultiIndex.dtypes to handle when no level names
State: closed

- [X] closes #38580 
- [X] tests added / passed
- [X] 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: Index
description: Related to the Index class or subclasses

Comments:
Hello @gurukiran07! 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-12-19 14:05:47 UTC
> We'll need a whatsnew (probably under 1.3)

the feature this PR fixes was added for 1.3 #37073, so release note probably not necessary.
thanks @gurukiran07
DOC:fix of DOCUMENTATION
State: closed

- [ ] closes #38311
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ Timedelta.isoformat DOC had duplicates i have deleted one of them. And their was two lines with wrong output that i have also fixed.] whatsnew entry


Labels:
name: Docs

Comments:
OK thanks. I'll make sure to Link the issue number. It was my first time therefore i didn't knew 



Thanks @Navron4500 - merging then following Simon's approval
TST: Bare pytest raises
State: closed

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


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

Comments:
Thanks @MJafarMashhadi 
This are base extension tests, which are also used by external projects. IMO, we should not be adding such specific message tests. I would propose to revert this change. 

(and @MJafarMashhadi, to be clear, not your fault!)
Makes sense

Should this be reverted and then ` ^pandas/tests/extension/` excluded from when the "disallow bare pytest raises" will be applied to CI, or should something generic like `match=''` be added here?
Yes, please revert that. I need to hardcode specific messages now in https://github.com/xhochy/fletcher/pull/212
Will do a revert tomorrow
@jorisvandenbossche There are 4 files that are changed in this PR, two of them are not in `extension/` should those be reverted too? 
The non-extension tests in `test_assert_series_equal.py` were not reverted in #38748
BUG: construction from dt64/td64 values with td64/dt64 dtype
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: Bug
name: Datetime
description: Datetime data dtype
name: Error Reporting
description: Incorrect or improved errors from pandas
name: Timedelta
description: Timedelta data type
name: Constructors
description: Series/DataFrame/Index/pd.array Constructors

Comments:
this is a sligth user facing change rigth? e.g. add a whatsnew
whatsnew added + greenish
TST: fix some mpl warnings
State: closed

- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

This change fixes `UserWarning` being emitted in tests calling `autocorrelation_plot`, `andrews_curves`, and `radviz`. 

`UserWarning` was caused by calling `plt.gca(xlim=..., ylim=...)` on a potentially already existing axis, so `matplotlib` warns about this forcing new axis creation. Solution was just to explicitly use `set_xlim` and `set_ylim` instead.

Does it make sense to also add to the relevant tests an assertion that no UserWarning is emitted to guarantee the new behavior?

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

Comments:
Will add tests once #38620 is figured out
> Will add tests once #38620 is figured out

hmm `tm.assert_produces_warning(None)`  should just work
> > Will add tests once #38620 is figured out
> 
> hmm `tm.assert_produces_warning(None)` should just work

It wasn't raising even when a warning was returned, with examples of this given in the issue. So there are currently no failures in master when wrapping these plotting calls with `tm.assert_produces_warning(None)`, even though a warning is returned
Wrapped existing calls that give warnings on master to avoid adding more plotting calls. If splitting out new tests for this is preferred instead, please let me know
> Wrapped existing calls that give warnings on master to avoid adding more plotting calls. If splitting out new tests for this is preferred instead, please let me know

this looks good; ping on green
Green
thanks @mzeitlin11 
@meeseeksdev backport 1.2.x
BUG: .sparse.to_coo() with numeric col index without a 0
State: closed

- [x] closes #18414
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Added tests for some other column name types which raised before this change, if that's overkill I can just test the int case from the OP.

Labels:
name: Bug
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Sparse
description: Sparse Data Type

Comments:
thanks @mzeitlin11 very nice, keep em coming!
TST: GH30999 Add match=msg to test_nat_comparisons_invalid
State: closed

This pull request xref #30999 to remove bare pytest.raises. It doesn't close that issue as I have only addressed one file: pandas/tests/scalar/test_nat.py . In that file there was only one test that had a bare pytest.raises and I added a message to the two instances of pytest.raises in that test. It required modifications to the test parameters in order to populate the message.

I did not add a whatsnew entry since it's only a tiny change to one test. Let me know if I should add one (and I am a bit unclear on how, i.e. what version this would end up in).

This is my first ever pull request to an open source project. I am expecting that I will have to make changes before it's accepted; thanks for your patience.

- [ ] xref #30999
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


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

Comments:
thanks @moink
REF: Block._astype defer to astype_nansafe in more cases
State: closed

Makes astype_nansafe for (td64|dt64) -> (object|str|string) match DTA/TDA/Series behavior.

Medium-term (weeks) the goal is to get rid of Block._astype altogether and just use astype_nansafe, which among other things will be helpful for ArrayManager.

This changes `Series[dt64].astype("string")` behavior in a way that causes a new xfail in test_astype_roundtrip, but as discussed in #36153 that test is already wrong for other reasons.

This also has a side-effect of changing Series(dt64, dtype="Sparse[object]") behavior, discussed in #38508 as possibly not-desirable.

Labels:
name: Refactor
description: Internal refactoring of code
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
> This also has a side-effect of changing Series(dt64, dtype="Sparse[object]") behavior, discussed in #38508 as possibly not-desirable.

where is this test case changed?
> where is this test case changed?

we dont have a test for this; #38508 introduced new ones
> This changes `Series[dt64].astype("string")` behavior in a way that causes a new xfail in test_astype_roundtrip, but as discussed in #36153 that test is already wrong for other reasons.

I'm not sure about the new behaviour, I think this should at least have a release note if not reverted.

old behaviour

```
>>> pd.__version__
'1.2.4'
>>> 
>>> tdi = pd.timedelta_range("1 Day", periods=3)
>>> ser = pd.Series(tdi)
>>> ser
0   1 days
1   2 days
2   3 days
dtype: timedelta64[ns]
>>> ser.astype("string")
0    1 days
1    2 days
2    3 days
dtype: string
>>> 
>>> dti = pd.date_range("2021", periods=3)
>>> ser = pd.Series(dti)
>>> ser
0   2021-01-01
1   2021-01-02
2   2021-01-03
dtype: datetime64[ns]
>>> ser.astype("string")
0    2021-01-01
1    2021-01-02
2    2021-01-03
dtype: string
```

new behaviour

```
>>> pd.__version__
'1.3.0.dev0+1567.g67c9385787'
>>> 
>>> tdi = pd.timedelta_range("1 Day", periods=3)
>>> ser = pd.Series(tdi)
>>> ser
0   1 days
1   2 days
2   3 days
dtype: timedelta64[ns]
>>> ser.astype("string")
0     86400000000000 nanoseconds
1    172800000000000 nanoseconds
2    259200000000000 nanoseconds
dtype: string
>>> 
>>> dti = pd.date_range("2021", periods=3)
>>> ser = pd.Series(dti)
>>> ser
0   2021-01-01
1   2021-01-02
2   2021-01-03
dtype: datetime64[ns]
>>> ser.astype("string")
0    2021-01-01T00:00:00.000000000
1    2021-01-02T00:00:00.000000000
2    2021-01-03T00:00:00.000000000
dtype: string
>>> 
```
@simonjayhawkins can u open a new issue - this is going to be very hard to revert - but i agree original behavior is correct so should fix 
#41409
Move docstring of NDFrame.replace in preparation of #32542
State: closed

This is a pre-cursor PR for #32542 as requested by @jreback [here](https://github.com/pandas-dev/pandas/pull/32542#issuecomment-744043100).

It moves the docstring of `replace()` method(s) to `pandas.core.shared_docs` so that we can reuse most of it for index classes.

Labels:
name: Docs

Comments:
thanks @oguzhanogreden
MultiIndex: support isna, fixes #34019
State: closed

- [x] closes #34019
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Note sure what to do with the (smoke) tests here. I suppose an explicit test comparing input and output would be good to have.


Labels:
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: MultiIndex
name: Needs Info
description: Clarification about behavior needed to assess issue
name: Stale

Comments:
Tested on top of 1.1.5, but seems there are additional failures on master.
Before putting in any additional time I'd like to know if the chosen approach is acceptable from a functional point of view, that is

>  If there is within a level an NA, the entire label is considered NA

is considered the correct approach.
> Before putting in any additional time I'd like to know if the chosen approach is acceptable from a functional point of view, that is
> 
> > If there is within a level an NA, the entire label is considered NA
> 
> is considered the correct approach.

Well ofc it's completely up to you! Bear in mind that this will need to go through multiple reviews and you will likely get feedback faster if you provide concrete examples of the implementation.

FWIW this seems not to break anything which is good. The one failure is an xfailed test which now doesn't raise:
``` 
pandas/tests/indexes/multi/test_integrity.py::test_isna_behavior
```
if you remove the xfail the checks should be green.

> this is not a good soln here. rather defining nunique on a MI is better

In the cython class?
> > this is not a good soln here. rather defining nunique on a MI is better
> 
> In the cython class?

no on MultiIndex
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. if you want to continue, pls ping and can re-open.