TST: add method/dtype coverage to str-accessor; precursor to #23167
State: closed

- [x] PRECURSOR to #23167
- [x] tests added / passed / xfailed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

The effort to test all methods of the `.str`-accessor against all inferred data types in #23167 unearthed a bunch of bugs. However, that PR is getting quite unwieldy, and since @jreback much prefers single-purpose PRs (and in the spirit of test-driven development), I'm splitting off just the addition of the parametrized fixtures / tests, with *lots* of `xfails` that will be removed by #23167 and subsequent PRs.


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Strings
description: String extension data type and string data

Comments:
Hello @h-vetinari! Thanks for updating the PR.

 - There are no PEP8 issues in the file [`pandas/conftest.py`](https://github.com/h-vetinari/pandas/blob/9b36a506f5d48116334da5affa95665395afe166/pandas/conftest.py) !

 - There are no PEP8 issues in the file [`pandas/tests/dtypes/test_inference.py`](https://github.com/h-vetinari/pandas/blob/9b36a506f5d48116334da5affa95665395afe166/pandas/tests/dtypes/test_inference.py) !

 - There are no PEP8 issues in the file [`pandas/tests/test_strings.py`](https://github.com/h-vetinari/pandas/blob/9b36a506f5d48116334da5affa95665395afe166/pandas/tests/test_strings.py) !



##### Comment last updated on November 20, 2018 at 06:48 Hours UTC
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23582?src=pr&el=h1) Report
> Merging [#23582](https://codecov.io/gh/pandas-dev/pandas/pull/23582?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/1f02bf240c3d0d3da338af868d056bfc169b28c2?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/23582/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/23582?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master   #23582   +/-   ##
=======================================
  Coverage   92.29%   92.29%           
=======================================
  Files         161      161           
  Lines       51500    51500           
=======================================
  Hits        47533    47533           
  Misses       3967     3967
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.69% <ø> (ø)` | :arrow_up: |
| #single | `42.42% <ø> (ø)` | :arrow_up: |


------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23582?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23582?src=pr&el=footer). Last update [1f02bf2...a53a28e](https://codecov.io/gh/pandas-dev/pandas/pull/23582?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

@jreback 
This is green, PTAL.
(note: strongly related to #23167, see OP)
> @jreback much prefers single-purpose PRs

Not just him, I think almost any of us who reviews PR's would prefer that if possible. 😉 
@gfyoung 
> Not just him, I think almost any of us who reviews PR's would prefer that if possible. 😉

That's fair. To me, #23167 is single purpose, but since the diff is large (and changing tests/code at the same time makes reviewing harder), I tried to find a way to slice it somehow.
@jreback 
> Thanks. How about moving this test [`test_str_accessor_api_for_categorical`] to `test_strings.py`? It's testing the API, but is clearly only related to the `.str` accessor.

Can I get a go-ahead on that? TBH, I don't think the `pandas/conftest.py` should be polluted with the very `.str`-specific methods. Would also make the unification much easier. Can move in separate PR, if you want.
FFFFFFFFFF. this is now the third PR infected by these spurious `ResourceWarnings` after #22225 and #23192. 

(and when I say "infected" I mean that this seems to depend on some bizarre combination of factors, but is at least stable across CI runs in the same PR).
@jreback 
I got sidetracked here with the hunt for those ResourceWarnings. I reverted those here now (but included #23706), so that the review here can continue. You can check that all the previous builds only failed in the travis 3.6 build (which has `PANDAS_TESTING_MODE="deprecate"` *plus* many tests that are otherwise skipped).

The warnings are clearly unrelated as this PR is not even changing any code! See also #23680.

Would like to get this PR moving so that the problems discovered by #23167 (and that PR itself) can be solved. PTAL
@gfyoung 
There's one timeout in travis and one in azure - could you restart them please?
@jreback 
Rebased without all the ResoureWarning stuff (will do that in a separate PR), and on top of #23706. PTAL :)
Opened #23726 for the crashed test (`pandas/tests/series/test_rank.py::test_pct_max_many_rows`) in the `travis-27` job that affected the last 3 builds. It bears reiterating that this PR does not change any code and cannot possibly be the source of that failure.
@jreback 
Out of all my open PRs, the one would be the most important to move forward. PTAL when possible.
@jreback Sorry for pinging again... This is blocking PRs for about 5-8 issues/PRs, so would very much like to get this moving. :)
I split off a good chunk with #23777. Could split off the dtype fixture as well, see https://github.com/pandas-dev/pandas/pull/23582#discussion_r234509825
@jreback 
Merged in #23777.
@jreback PTAL :)
(can split off dtype fixtures as separate PR, see comment in the diff)
@jreback 
> @h-vetinari i know we have gone back and forth a few times. but the current state is just not great.

It's OK, some things take time. This has had one round of review and one split-off precursor so far, so not *too* bad either.

> let's duplicate the fixtures, the full set in test_inference and then pick the ones you need in test_strings.
i will have followup comments after.

Not sure I understand - the fixtures are in `pandas/conftest.py`, so don't need duplication in `test_inference`.  I've removed the conftest-import now and duplicated the *subset*  of inferred dtypes that pass the constructor in `test_strings.py`.
@jreback PTAL
thanks @h-vetinari 

would like to remove the xfails (e.g. fixing) as soon as can be done.