BrandonFors/pandas-github-issues huggingface dataset:
[ENH] Add "fullmatch" matching mode to Series.str [#32806]
State: closed

- [X] closes #32806
- [X] tests added / passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry

This is my first PR against this project, so apologies if I've missed any steps in the process. I'm assuming that I'm supposed to fill in the checklist above myself.

This pull request adds the `fullmatch` regular expression matching mode to the other modes already present under the `Series.str` namespace. For example:

```python
>>> s = pd.Series(["foo", "bar", "foobar"])
>>> s.str.fullmatch("foo")
0     True
1    False
2    False
dtype: bool
```

The `fullmatch` matching mode restricts matches to those that only match the *entire* string. Note the differences from `match`:
```python
>>> s = pd.Series(["foo", "bar", "foobar"])
>>> s.str.fullmatch("foo")
0     True
1    False
2    False
dtype: bool
>>> s.str.match("foo")
0     True
1    False
2     True
dtype: bool
```

I've also added regression tests and a "what's new" entry.

I have also opened issue #32806 to cover this new feature.

Labels:
name: Strings
description: String extension data type and string data

Comments:
I'm currently rerunning CI tests locally after fixing that linter error. Sorry about that; I didn't know that `black` doesn't reorder imports for you.
Can you merge master one more time? Should resolve CI failure
Merged in changes from master; waiting for the next batch of CI results to come back.
Pushed some changes late last night that should address the remaining issues.
thanks @frreiss very nice!
PERF: Using Numpy C-API when calling `np.arange`
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

---

Somewhat of a follow up to #32681.

---

I could not benchmark this change as this function is ```cdef``` and not ```def``` or ```cpdef``` (I would also love if someone could give me a tip on how to benchmark  ```cdef``` functions from an ```Ipython``` shell for example).

What I did what I ran ```cython -a pandas/_libs/internals.pyx``` and took a screenshot of the before and after.

---

#### Master:

![orig_arange](https://user-images.githubusercontent.com/50263213/76964675-c4885280-692b-11ea-8ddf-b9eaf2ed0cd2.png)

---

#### PR:

![New_arange](https://user-images.githubusercontent.com/50263213/76964722-d7028c00-692b-11ea-970a-454cbb56adcf.png)


---

Also, is there a reason not to replace every call of ```np.arange``` with ```cnp.PyArray_Arange```? (cython files only)

Labels:
name: Performance
description: Memory or execution speed performance

Comments:
> I would also love if someone could give me a tip on how to benchmark cdef functions from an Ipython shell for example

i usually write a `def wrapper` in the same file to call the cdef function in a loop.  You could also find the "closest" non-cdef function that calls the affected function and profile that
> Also, is there a reason not to replace every call of np.arange with cnp.PyArray_Arange? (cython files only)

if its in a place that gets called a zillion times (especially in a loop), then go for it.

For others is probably isn't worth the cognitive overhead for the next reader to come along.
> > Also, is there a reason not to replace every call of np.arange with cnp.PyArray_Arange? (cython files only)
> 
> if its in a place that gets called a zillion times (especially in a loop), then go for it.

Okay, make sense.

> For others is probably isn't worth the cognitive overhead for the next reader to come along.

If a comment is included (like in this PR), does it solves the cognitive overhead?

and are the there any other objections?

> If a comment is included (like in this PR), does it solves the cognitive overhead?

I guess, but then it feels fishy to be adding identical comments in many places (then again, I do this too).  Your call.

Separate topic: in core.groupby.generic L389, L1350, L1702 there are calls to ._convert that I _think_ are superfluous.  Getting a second pair of eyes to try to reason about the code and prove it would be great if you're up for it.
thanks @MomIsBestFriend 
> I guess, but then it feels fishy to be adding identical comments in many places (then again, I do this too). Your call.

BTW, I don't think we should merge such PRs in the future without a performance check showing an considerable improvement. 
(as I agree it gives cognitive overhead, so we should only do it when worth it; what is worth it is of course subjective, but at least we should time it)
CLN/STY: pandas/_libs/internals.pyx
State: closed

- [ ] 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
name: Clean

Comments:
thanks @MomIsBestFriend
CLN: remove DatetimeLikeArray._add_delta
State: closed

Ultimately we want to 1) avoid using _from_sequence (so we can make that stricter in what it accepts) and 2) be more careful about when we use `freq="infer"`.  Splitting up the cases handled by _add_delta will make this more feasible.

Labels:
name: Timedelta
description: Timedelta data type
name: Clean

Comments:
thanks. I think could type these functions pretty easily now :->
BUG/API: _values_for_factorize/_from_factorized round-trip
State: closed

- [x] closes #32673
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

having this round-trip-ability is necessary for implementing an efficient general-case broadcast_to/tile compat method.

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

Comments:
@jorisvandenbossche objections?
Yes, see my comment, we should first discuss further in the issue before simply changing
mothballing
BUG: created check and warning for merging dataframes on unequal inde…
State: closed

…x levels (#13094)

- [ ] closes #13094
- [ ] tests added / passed
- [ ] warn users when merging between different index levels


Labels:
name: Bug
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Error Reporting
description: Incorrect or improved errors from pandas

Comments:
Hello @SeeminSyed! 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/frame/test_axis_select_reindex.py`](https://github.com/pandas-dev/pandas/blob/7a9e0fede74e09ae8caaa41a00809c748a75c69f/pandas/tests/frame/test_axis_select_reindex.py):

> [Line 186:9](https://github.com/pandas-dev/pandas/blob/7a9e0fede74e09ae8caaa41a00809c748a75c69f/pandas/tests/frame/test_axis_select_reindex.py#L186): [E265](https://duckduckgo.com/?q=pep8%20E265) block comment should start with '# '
> [Line 191:13](https://github.com/pandas-dev/pandas/blob/7a9e0fede74e09ae8caaa41a00809c748a75c69f/pandas/tests/frame/test_axis_select_reindex.py#L191): [E128](https://duckduckgo.com/?q=pep8%20E128) continuation line under-indented for visual indent
> [Line 208:1](https://github.com/pandas-dev/pandas/blob/7a9e0fede74e09ae8caaa41a00809c748a75c69f/pandas/tests/frame/test_axis_select_reindex.py#L208): [W293](https://duckduckgo.com/?q=pep8%20W293) blank line contains whitespace


Thanks @SeeminSyed closing as stale. ping to re-open.
BLD: Suppressing warnings when compiling pandas/_libs/writers
State: closed

- [x] ref #32163
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

---

This is the error that this PR is getting rid of:

```
pandas/_libs/writers.c: In function ‘__pyx_pw_6pandas_5_libs_7writers_1write_csv_rows’:
pandas/_libs/writers.c:3435:15: warning: ‘__pyx_v_j’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 3435 |     __pyx_t_1 = (__pyx_v_j + 1);
      |     ~~~~~~~~~~^~~~~~~~~~~~~~~~~
pandas/_libs/writers.c:2911:14: note: ‘__pyx_v_j’ was declared here
 2911 |   Py_ssize_t __pyx_v_j;
      |              ^~~~~~~~~
```

Labels:
name: Build
description: Library building on various platforms

Comments:
LGTM pending green
Thanks @MomIsBestFriend
ERR: Raise on invalid na_action in Series.map
State: closed

- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

`Series.map` treats any `na_action` other than `"ignore"` like `None`, but it should probably raise if someone provides an invalid option:

```python
In [1]: import pandas as pd                                                                                                                                

In [2]: pd.Series([1, 2, 3]).map(lambda x: x, na_action="xxxxx")                                                                                           
Out[2]: 
0    1
1    2
2    3
dtype: int64
```

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

Comments:
Do any Index methods need the same fix?
> Do any Index methods need the same fix?

Looks like they do. Actually it seems worse, e.g. `ExtensionIndex` just ignores `na_action`: https://github.com/pandas-dev/pandas/blob/master/pandas/core/indexes/extension.py#L291

Edit: Tried fixing this and wasn't able to without introducing more bugs. Opened this issue instead: https://github.com/pandas-dev/pandas/issues/32815.
> lgtm. merge master and ping on green.

@jreback Merged master and green, thanks for reviewing
thanks
TST: Define sections in pandas/conftest.py
State: closed

part of #31989
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`



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

Comments:
Apart from one helper function rename and a new docstring (marked with a comment), this is purely a reordering.

I suggest to just read the new `pandas/conftest.py` instead of the "Files changed" tab
@jreback CI isn't green, but the failure isn't related. The same check fails for a lot of other PRs. -> I think this is good to be merged
Thanks @SaturnFromTitan
CLN: .value -> ._values outside of core/
State: closed

pandas.plotting I left alone

Labels:
name: Clean

Comments:
+1 on this, I agree we should not use `.values` at all; agree that using ``np.asarray()`` when we actually need a numpy arra (or .to_numpy()) explicitly is nice too. In an ideal world, we *never* should use `.values` and we can then *maybe* deprecate on the user side.
@jbrockmendel I am fine with the general rule of not using `.values`. Although I think in cases where we explicitly need a numpy array, we should rather use `asarray` / `to_numpy`.

But there are still some relevant inline comments
@jorisvandenbossche updated the one clearly-actionable inline comment.  can you point out which other comments you have in mind?  (or "resolve conversation" for the complement)
Added new comments to those that are IMO still actionable, and resolved the others
thanks
CLN: Split up Boolean array tests
State: closed

Apologies for the size of this PR, but it looks like the ExtensionArray tests sometimes have their own directories by type, and other times all tests for the type are in a single file (e.g., https://github.com/pandas-dev/pandas/tree/master/pandas/tests/arrays/categorical vs. https://github.com/pandas-dev/pandas/blob/master/pandas/tests/arrays/test_boolean.py). I think the "whole directory for type" structure makes it easier to find and add new tests, so I'm doing that here for the Boolean tests. There shouldn't be any changes other than moving things around.

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

Comments:
Personally, I don't find that this makes it easier to find the tests, though ... (with this I would need to look in multiple files of which I don't know the name)

Now, I assume that if we keep adding tests here, splitting it becomes unavoidable. 

The question might then also be: where to put the line on what should be tested here? 
For example, @jbrockmendel mentioned parametrizing the op tests to also test with series and dataframe. But then those tests maybe rather belong in one of the other test modules that are more specific, like tests/arithmetic, tests/reductions/, ...?

> mentioned parametrizing the op tests to also test with series and dataframe. But then those tests maybe rather belong in one of the other test modules that are more specific, like tests/arithmetic, tests/reductions/, ...?

Yes, if that was unclear: if the arithmetic/reduction tests can/should be parametrized over EA/Series/DataFrame, then they likely should be moved to tests.arithmetic or tests.reductions etc.  (the situation is slightly complicated by the fact that those tests are also parametrized over Index which these can't be until ExtensionIndex is in operation)
Curious if @jreback has any preferences on the layout of these tests?
once a file gets > 500 lines, splitting into sub-modules is really needed. we have already split most other tests, so this is fine.
CLN: .values -> ._values
State: closed


Labels:
name: Clean

Comments:
> what logic do we we use to decide when to use which property?

The default should be ._values, as .values may be either lossy (dt64tz), involve an object-dtype cast (period, interval), or be surprising (until recently I forgot that Series[Sparse].values gives a SparseArray, not ndarray).

Both this and #32781 are aimed at usages where the two are equivalent.  This should make it easier to focus attention on usages where they are not equivalent, and double-check whether those are intentional or improveable.
comments here: https://github.com/pandas-dev/pandas/pull/32781#issuecomment-600927742
Travis yellow is wrong; travis is green
kk, no test covers this?
> no test covers this?

not sure what you're referring to
TYP: PandasObject._cache
State: closed

Move ``_cache: Dict[str, Any]`` fragment to PandasObject, where it belongs + related changes.

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

Comments:
Just out of curiosity why do you think this is better placed in PandasObject?
My motivation is to have it set on the same class as _reset_cache is set on, because those two belong logically together. This would also prevent that _cache attribute is set on a child class that is incompatible with _reset_cache.
thanks @topper-123
CLN: Consolidate numba facilities
State: closed

- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Precursor to https://github.com/pandas-dev/pandas/issues/31845 and other numba engine additions, creates `pandas/core/numba_.py` (open to move elsewhere) as a shared place for common numba operations like default parameters and jitting functions.

Labels:
name: Clean

Comments:
Maybe better placed in pandas/core/util? @jreback 
Good point. I can move it to util. 
seems good enough place for now. we can see how / if this grows and move around later. e.g. this is conceptually like pandas/core/computation stuff (though not nearly as big).
TYP: annotate Block/BlockManager putmask
State: closed

medium-term goal is to avoid passing Series/DataFrame objects to Block methods via BlockManager.apply

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

Comments:
@simonjayhawkins i lost track, is there anything left actionable here?
rebased+green
needs another rebase
rebased+green
thanks @jbrockmendel
REF: Implement core._algos
State: closed

ATM core.algorithms and core.nanops are a mish-mash in terms of what inputs they expect.  This implements core._algos directory intended for guaranteed-ndarray/EA-only implementations.

For the first function to move I de-duplicated a `shift` method.  Need suggestions for what to call this module.

Labels:
name: Refactor
description: Internal refactoring of code
name: Algos
description: Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff

Comments:
> what other functions do you envision to put here?

1) EA-compatible variants of np.broadcast_to, np.tile, (maybe np.putmask?), etc will likely have their own file.
2) Things like `shift` where we can pull the meat of the method out of internals.
3) Some parts of algorithms and nanops that are array-specific
    - Some of the functions in these files seem like they are doing Index/Series unwrapping haphazardly.  By separating out the array-specific parts, we can ensure we do those unwrappings just once.

Renamed "_algos" -> "array_algos"
thanks @jbrockmendel the above sounds like a good plan
@jorisvandenbossche another candidate for array_algos would be most of what is currently in dtypes.concat, which you and i agreed was a weird place for those functions
Yep, now another option for those concat things would be to move each of them to their respective array implementation (but that might depend a bit on how we rework the EA interface regarding concatting)
> move each of them to their respective array implementation

I like that idea quite a bit
BUG: Allow list-like in DatetimeIndex.searchsorted
State: closed

- [x] closes #32762
- [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

Comments:
> ideally as a followup we test things like IntervalIndex as well for this behavior (pls create an issue)

Created https://github.com/pandas-dev/pandas/issues/32845
> this seems to overlap with #32874 can you clarify?

Yeah, that's me confusing myself working on duplicative branches. I could try merging that one into here, hope I don't break things, and then close the other (it was originally only intended to add testing code)?
> > this seems to overlap with #32874 can you clarify?
> 
> Yeah, that's me confusing myself working on duplicative branches. I could try merging that one into here, hope I don't break things, and then close the other (it was originally only intended to add testing code)?

yep one branch is fine (tests & changes)
thanks @dsaxton 

I think we need to collect the index searchsorted tests and separate them out, but a separate exercise.
BUG: Respect check_dtype in assert_series_equal
State: closed

- [x] closes #32747
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

I'm not sure if https://github.com/pandas-dev/pandas/blob/master/pandas/_testing.py#L1174 needs to change too?

cc @jorisvandenbossche 

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

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-03-17 18:01:47 UTC
can you add a whatsnew note (can put in bug fixes other section)
> can you add a whatsnew note (can put in bug fixes other section)

IIUC the bug is only on master, not released. Is the whatsnew needed?
> IIUC the bug is only on master, not released. Is the whatsnew needed?

Part of the fix did fix something compared to 1.0 (eg for intervals)
Thanks @dsaxton !
> Part of the fix did fix something compared to 1.0 (eg for intervals)

If this fixes something that worked in 1.0, then should this be backported?
Sorry, I meant: it's partly fixing a bug that is in 1.0 (hence the whatsnew note is fine), and partly a regression that is only in master (which doesn't need a whatsnew). 
But the bug from 1.0 is not a regression nor critical, so shouldn't need a backport
ah. my bad.
copy license text from: tidyverse/haven
State: closed

The existing HAVEN_LICENSE file was lacking any form of actual license.
After checking the pandas source, it appears that the reference to HAVEN
is in relation to tidyverse/haven, copying the license from
tidyverse/haven for clarity.

Signed-off-by: Jamin Collins <jamin@amazon.com>


Labels:
name: Admin
description: Administrative tasks related to the pandas project

Comments:
For reference, can you dig up the commit in haven adding their license? When we added the test files, Haven apparently didn't include a license yet: https://github.com/pandas-dev/pandas/pull/26537#issuecomment-496671885
Looks like the license was added as a result of this PR:
https://github.com/tidyverse/haven/pull/484

and this commit:
https://github.com/tidyverse/haven/commit/b2410adc3b6428c549452e7b4015e436a31fb4d5
The PR to add here predates the change to add the license, so I don't think should be retro-fitting that here, no?
It was noted in this comment https://github.com/pandas-dev/pandas/pull/26537#issuecomment-496671885, that the project (haven) appeared to be under MIT.  The next comment https://github.com/pandas-dev/pandas/pull/26537#issuecomment-496678901 indicates that (hadley) is fine with that.

The PR that I reference https://github.com/tidyverse/haven/pull/484 was merged by what appears to be the same individual.

However, the files in this project do no reflect the MIT license.  This is simply an attempt to clarify what was (apparently) already recorded separately.
this is fine. cc @TomAugspurger 
Yep, LGTM. I just wanted the verification that we didn’t fail to copy the license initially. 

> On Mar 16, 2020, at 21:46, Jeff Reback <notifications@github.com> wrote:
> 
> ﻿
> this is fine. cc @TomAugspurger
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or unsubscribe.

Thanks @jamin-aws-ospo !
REF: avoid _internal_get_values in json get_values
State: closed

cc @WillAyd 

As mentioned in #32731, this changes the behavior for CategoricalIndex[dt64tz] and Series with that for its index.  Suggestions for where tests for that should go?

Note: the Categorical[dt64tz] cases still do a trip through object-dtype; I don't want to futz with that in C-space.

Labels:
name: IO JSON
description: read_json, to_json, json_normalize
name: Clean

Comments:
> Suggestions for where tests for that should go?

Pretty much everything right now goes into `test_pandas`; there's a PR to start fixturing / splitting things that I promise I'll get back to...but for now just place there
lgtm. happy if you guys are.
Thanks @jbrockmendel
DOC: start 1.0.3
State: closed


Labels:
name: Docs

Comments:
Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

```
$ git checkout 1.0.x
$ git pull
```

2. Cherry pick the first parent branch of the this PR on top of the older branch:
```
$ git cherry-pick -m1 7aa3dd274475245f8268a78d45ef65de379e31c0
```

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

```
$ git commit -am 'Backport PR #32746: DOC: start 1.0.3'
```

4. Push to a named branch :

```
git push YOURFORK 1.0.x:auto-backport-of-pr-32746-on-1.0.x
```

5. Create a PR against branch 1.0.x, I would have named this PR:

> "Backport PR #32746 on branch 1.0.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to [suggest an improvement](https://github.com/MeeseeksBox/MeeseeksDev).