BrandonFors/pandas-github-issues huggingface dataset:
BUG: string methods with NA
State: closed

- [x] closes #31632 
- [ ] 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
name: Strings
description: String extension data type and string data

Comments:
Have added `_typ` to class `NAType` so it can be identified in `is_nan()` correctly. That's one way of doing it but probably can be done better. Will update the PR according to suggestions.
I don’t think we want is_nan to recognize NA.
Agreed. To fix #31632 we'll likely want to adjust how the `na_map` function is called.
thanks for the inputs...will update this PR or raise a new one
@TomAugspurger  I think `vec_binop` is called instead of `na_map` because input is a vector. Should we implement a variant of `vec_binop` which can handle NA? Or  a shorter fix might be checking for NA before calling `bytes.__mul__` and `str.__mul__`
Either of those sound sensible.

On Wed, Feb 5, 2020 at 10:51 PM Prakhar Pandey <notifications@github.com>
wrote:

> @TomAugspurger <https://github.com/TomAugspurger> I think vec_binop is
> called instead of na_map because input is a vector. Should we implement a
> variant of vec_binop which can handle NA? Or a shorter fix might be
> checking for NA before calling bytes.__mul__ and str.__mul__
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/31684?email_source=notifications&email_token=AAKAOIS2IMDWKD53WV5ZOC3RBOJNBA5CNFSM4KQFW7K2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK55RWQ#issuecomment-582736090>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAKAOIVMIKLXCXMXQPNSXY3RBOJNBANCNFSM4KQFW7KQ>
> .
>

@jreback ~~i think other string methods will be fine because most of them use `_na_map` which handles `pd.NA`. This specific issue was due to `str_repeat` using `vec_binop` for vectored inputs which didn't handle `pd.NA`. Let me know if you want the tests anyway.~~ Will add tests for NA
@jreback should i put the pd.NA tests in separate function or just add to functions that are already there?
@Meeseeksdev backport to 1.0.x
Something went wrong ... Please have  a look at my logs.
REGR: fix op(frame, frame2) with reindex
State: closed

- [x] closes #31623
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

cc @TomAugspurger this is pretty ugly, and I'm not sure how well it will behave if either frame has MultiIndex colums.

On the plus side, it could improve perf in the many-columns-but-small-intersection case.

The ugliness might be improved by moving this check to before the _align_method_FRAME call

Labels:
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations

Comments:
@jbrockmendel is this still a POC? Or is this good for 1.0.2?
> is this still a POC? Or is this good for 1.0.2?

Borderline.  Do we have a timeline for 1.0.2?
Next week-ish?
> Next week-ish?

Sounds good.  I'd like to try a second draft of this, if we're OK with this approach.
The approach looks generally OK to me. 
But for possible alternative approach: do you know why this started failing? What changed in the ops code that might have caused this?
@jbrockmendel What's the status of this?
I opened https://github.com/pandas-dev/pandas/issues/31874 for a larger discussion on how to fix this, which addresses Jeff's comment in https://github.com/pandas-dev/pandas/pull/31679/files#r376807594. That's too large to do now though. 
> What's the status of this?

I'm catching up on reviewing things, hope to push a second draft of this later today.  I don't expect the logic to change, just the organization.
Updated, separating out the new logic to dedicated functions, doing it before the _align_frame_METHOD call.

looks good, some comments
@jbrockmendel couple of comments.
I'm under the weather, will get to these once I'm back on my feet.
> I'm under the weather, will get to these once I'm back on my feet.

sure np! feel better!
comments addressed i think
thanks @jbrockmendel 
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 f4dc9f9028a3da539126f9a8a37e7c41fc7b4b3c
```

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

```
$ git commit -am 'Backport PR #31679: REGR: fix op(frame, frame2) with reindex'
```

4. Push to a named branch :

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

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

> "Backport PR #31679 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).
REGR: Fixed handling of Categorical in cython ops
State: closed

Fixed by falling back to the wrapped Python version when the cython
version raises NotImplementedError.

Closes https://github.com/pandas-dev/pandas/issues/31450

Labels:
name: Groupby
name: Regression
description: Functionality that used to work in a prior pandas version
name: Categorical
description: Categorical Data Type

Comments:
LGTM.  Is this the long-term solution?
I'm not really sure to be honest. 
this looks good. it should be the long-term, until we fully handle EAs in cython.
BUG: fixes bug when using sep=None and comment keyword for read_csv
State: closed

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


Labels:
name: Bug
name: IO CSV
description: read_csv, to_csv

Comments:
When adding `dict(sep=None)` as parameter to `test_line_comment` I found that my current fix does not work if the first line is a comment.

~~Overall I think the flow of comment removal and separating columns is a bit inconsistent. During searching for the bug I found that `PythonParser._check_comments` is sometimes called before separating the line (e.g. when sniffing the separator) and sometimes afterwards. In my opinion it would make more sense  to remove comments first in all cases.~~

The next commit fixes the issue with comments on the first line.
The Travis CI build failed with the following message:
```
UnavailableInvalidChannel: The channel is not accessible or is invalid.

  channel name: c3i_test

  channel url: https://conda.anaconda.org/c3i_test

  error code: 404
```

Is there a way to rerun it?
@s-scherrer : This is actually an unrelated error that we need to resolve on `master`.

cc @pandas-dev/pandas-core
thanks @s-scherrer
BUG: Block.iget not wrapping timedelta64/datetime64
State: closed

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


Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves

Comments:
k, i guess this was because we were always boxing
@MeeseeksDev backport to 1.0.x
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 1996b17599731b889895b0e1edf758588c068fbb
```

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

```
$ git commit -am 'Backport PR #31666: BUG: Block.iget not wrapping timedelta64/datetime64'
```

4. Push to a named branch :

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

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

> "Backport PR #31666 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).
Replaced .format with f- strings
State: closed

xref #29547
for
- [ ] pandas/util/_print_versions.py
- [ ] pandas/tests/tseries/frequencies/test_to_offset.py


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

Comments:
Please can you use 1 PR and push the commit to the same branch? https://github.com/pandas-dev/pandas/pull/31627

Thanks
#31627 is closed now
There is only one PR
thanks @leandermaben 
Thank you so much @jreback
check first and last points' labels are correct
State: closed

- [ ] closes #31580 
- [ ] 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: Visualization
description: plotting

Comments:
@jbrockmendel seems this worked, thanks for your suggestion!
thanks @MarcoGorelli
API: allow step!=1 slice with IntervalIndex
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

See discussion https://github.com/pandas-dev/pandas/pull/16386#discussion_r374807689

cc @jorisvandenbossche @jschendel 

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Interval
description: Interval data type

Comments:
I suppose should add a whatsnew bug fix entry.
> what would you expect for [...] `s[1.5:9.5:2]` [...] I think that is less clear, which might be the reason it was disallowed initially.

I agree that is less obvious, as that is not positional.
> I agree that is less obvious, as that is not positional.

So let's not allow that?


> So let's not allow that?

OK I see, I thought that the float-slice would have been disallowed somewhere else, but evidently not.  So yes, this needs to be updated to restore that.

@jorisvandenbossche double-checking: suppose we get integer sllce that is valid for either loc or iloc:

```
import pandas as pd
import numpy as np

ser = pd.Series(np.arange(5), pd.IntervalIndex.from_breaks(np.arange(2, 14, 2))) 
key = slice(3, 6)

ser[key]
ser.loc[key]
ser.iloc[key]

key2 = 3
ser[key2]
ser.loc[key2]
ser.iloc[key2]
```

For the slice we treat the ambiguous `__getitem__` key as positional, while for the int we treat it as a label.  That's on purpose, right?
Closing to clear the queue (in particular #31786 should happen before this), will revisit
can you rebase. look ok to me. @jschendel 
rebased+green
can you rebase. 
rebased+green
thanks 2 followups:

1) @jorisvandenbossche comment about having start/stop as intervals, pls create an issue
2) does this need an example in the docs?
REF: call _convert_scalar_indexer upfront in Series.__getitem__
State: closed

This simplifies a try/except block and moves us towards being able to tighten what we accept in _convert_scalar_indexer among others.  I think we'll soon be able to get rid of the kludge on L866/868.

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Clean

Comments:
rebase just to make sure didn't break things as merged in odd order tonight (merge on green).
green
ENH: Timestamp constructor now raises more explanatory error message
State: closed

Timestamp constructor now raises explanatory error message when year, month or day is missing

- [x] closes #31200
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


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

Comments:
The error message of `datetime.datetime()` seems to depend on its version. On my computer I have `TypeError: function missing required argument 'year' (pos 1)`, and in some of the CI environment as well as in @mroeschke 's comment in the original issue (#31200) the error is `TypeError: Required argument 'month' (pos 2) not found`. For compatibility I decided to use `try-except` block to extract the message from `datetime.datetime()` in the test.

The use of `re.escape` to match exact string in `pytest` is described in their official documentation.
@fujiaxiang do you have a measure on how performance is affected?  We don't call the constructor this way often, so a small hit is OK.
> @fujiaxiang do you have a measure on how performance is affected? We don't call the constructor this way often, so a small hit is OK.

@jbrockmendel I ran %timeit on current master and this implementation, this is what I get:
```python
# on current master
>>>%timeit pd.Timestamp(year=2020, month=1, day=1)
1.19 µs ± 4.74 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

# re-built from this PR
>>>%timeit pd.Timestamp(year=2020, month=1, day=1)
1.7 µs ± 2.21 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
```

Looks like it takes about 43% longer with this PR.
Let me know if you guys need a more accurate way of performance testing.
thanks
BUG: avoid specifying default coerce_timestamps in to_parquet
State: closed

Looking into the usage question of https://github.com/pandas-dev/pandas/issues/31572, I noticed that specifying the version to allow writing nanoseconds to parquet worked in plain pyarrow code, but not with pandas' `to_parquet`. 
This is because we hardcode `coerce_timestamps="ms"` while the default is `None`, which has version-dependent behaviour (eg if version="2.0", actually write the nanosecond data)

Labels:
name: Bug
name: IO Parquet
description: parquet, feather

Comments:
thanks @jorisvandenbossche 
For us this change caused a nasty bug after upgrading from pandas 1.0.3 to 1.1.x. Apparently AWS Athena/Presto doesn't support nanosecond precision, so our timestamps started appearing with year 52000 when we created the parquet files using pandas 1.1.x. Just FYI for anyone else having the same issue. Adding `coerce_timestamps="ms"` (the previous default) to the `to_parquet()` call fixes the issue.
@mnylen FWIW, since recently Presto does support nanosecond precision for date/time types, including timestamps. (see https://github.com/prestosql/presto/issues/1284 for more info).
However, this seems to be a different problem. If you can time, would you be able to answer a question in https://github.com/prestosql/presto/issues/4662#issuecomment-667583564 so that we know what needs to be improved?
@findepi I mentioned Presto only because AWS Athena uses that underneath. Athena is based on Presto 0.172, so it's likely that even if Presto nowadays supports nanoseconds for timestamps, it'll take a long time before Athena upgrades to the newer version.
PERF: Cache MultiIndex.levels
State: closed

Closes https://github.com/pandas-dev/pandas/issues/31648.

This should have been caught by the benchmark MultiIdnex.time_index_slice. Confirming that now..

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Performance
description: Memory or execution speed performance
name: MultiIndex

Comments:
LGTM
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 3b2b0d31ad5bfc004e7ab5bb772ea4100d5bebda
```

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

```
$ git commit -am 'Backport PR #31651: PERF: Cache MultiIndex.levels'
```

4. Push to a named branch :

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

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

> "Backport PR #31651 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).
                
Ah, just realized I forgot a release note, sorry.

Will push that and handle the backport in a bit.
DOC: Removed numeric_only parameter from pd.DataFrame.mad docs
State: closed

- closes #29079 


Labels:
name: Docs

Comments:
Please review this @MarcoGorelli @mroeschke 
I think it's fine, could you include a screenshot of the documentation?

Inside the `doc` folder, run
```
python make.py clean
python make.py --single pandas.DataFrame.mad
```

(see [building the documentation](https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#building-the-documentation))
![Screenshot from 2020-02-05 04-59-59](https://user-images.githubusercontent.com/39258575/73796968-bf849f00-47d4-11ea-844b-045458f298c1.png)

Hi, I'm getting the following error when I run "python make.py clean"
When I ran "python3 make.py clean", I got an error saying "ModuleNotFoundError: No module named 'pandas._libs.tslibs.conversion' "
@r0cketr1kky you'll need to build the c extensions: https://pandas-docs.github.io/pandas-docs-travis/development/contributing.html#creating-a-python-environment
Documentation screenshot @MarcoGorelli : 

![Screenshot from 2020-02-05 06-37-33](https://user-images.githubusercontent.com/39258575/73802064-6a4f8a00-47e2-11ea-9158-919b36cd1c03.png)

P.S. Thanks for the link @mroeschke . This helped me.
Please review this!
How long does it take to merge this PR?
@r0cketr1kky I don't have write-access to the repo, and I presume that those who do are prioritising other work. Please be patient, I'm pretty sure this will get merged
Sure, thanks a lot for assisting me!
thanks @r0cketr1kky 

if you'd canvas the other operations that use ``_num_doc`` to see if any need changing would be great.
Ahh I just saw this comment @jreback I'm having a busy semester :(
Nevertheless thank you @Bharat123rox for this commit :+1:
CLN: prelims for MultiIndex.get_value
State: closed

Unravelling MultiIndex.get_value and get_loc is shaping up to be a PITA.  This separates out the easiest parts of the get_value cleanup.

@TomAugspurger can you confirm that the change made here to the `__finalize__` call is correct?  If so, suggest a test?


Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: MultiIndex
name: Clean

Comments:
needs a rebase, and now that I think about it, try_mi really should NOT be where it is, it actually should be a series method i think.
> try_mi really should NOT be where it is

I'm hoping we can get it into `_get_values_for_loc`, separate PR
Closing in favor of #31736.
CLN: Replace format with f-strings in test_repr_info
State: closed

Updated `pandas/tests/frame/test_repr_info.py` to use f-strings

ref: https://github.com/pandas-dev/pandas/issues/29547

- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`


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

Comments:
thanks!
REF: call _maybe_cast_indexer upfront, better exception messages
State: closed

This will actually allow us to remove e.g. CategoricalIndex.get_loc by moving the 2 relevant lines into CategoricalIndex._maybe_cast_indexer.  Large parts of DTI/TDI/PI.get_loc will also be simplifiable.

We've got both Index._maybe_cast_indexer and Index._convert_scalar_indexer which hopefully we'll only need one of.

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves

Comments:
sure
REF: implement ExtensionIndex._concat_same_dtype, use for IntervalIndex
State: closed


Labels:
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

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 2020-02-08 02:13:36 UTC
cc @jorisvandenbossche 
thanks @jbrockmendel
Replace .format with f-strings
State: closed

- [ ] solves few cases of  #29547



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

Comments:
Hello @leandermaben! 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-02-04 17:30:48 UTC