BrandonFors/pandas-github-issues huggingface dataset:
PR06 doc string fixes
State: closed

This PR contains doctrine PR06 fixes mostly doing the conversion below(not that many left to fix :)):

boolean to bool

string to str

integer to int

Tests unchanged

black pandas ran successfully

Labels:
name: Docs

Comments:
Thanks @tolaa001 . can you merge master. some of these look familiar.
Hello @tolaa001! 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 2019-10-13 17:49:47 UTC
Thanks @tolaa001
BUG: Fix comparison between nullable int and string
State: closed

- [x] closes #28930
- [x] tests added / passed
- [x] passes `black pandas`
- [x] whatsnew entry

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

Comments:
The fix here makes sense.  Are there any other types we could compare against that this fixes?  e.g. `arr == datetime.now()`?
Merging master should fix the CI failures.
The CI failures seem to be from tests on cummin and cummax for Series with datetime64 and timedelta64 dtypes
@dsaxton can you fix up merge conflict?
thanks @dsaxton 
> thanks @dsaxton

NP, thanks for the review
TST: add test_series_any_timedelta for GH17667
State: closed

Test case example is the same as the one given in the issue #17667 

- [x] closes #17667 
- [x] 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

Comments:
Thanks @rohitsanj 
@rohitsanj feel free to contribute more tests like this! https://github.com/pandas-dev/pandas/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22+label%3A%22Needs+Tests%22
fix #28926 pandas\api\test_api.py mypy errors
State: closed

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


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

Comments:
Hello @yogendrasoni! 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 2019-10-12 03:46:05 UTC
> @yogendrasoni lgtm @WillAyd

@WillAyd  , @simonjayhawkins simonjayhawkins anything else is required ?
Thanks!
REF: de-duplicate groupby_helper code
State: closed

There's one other piece of de-duplication I think should be feasible but cython is still raising compilation errors for, so will do separately.

Orthogonal to #28931, but I expect it will cause merge conflicts.  28931 should be a higher priority.

Labels:
name: Groupby
name: Clean

Comments:
lgtm any perf implications?
Rebased + green.  
Thanks @jbrockmendel
TYPING: Module 'pytz' has no attribute 'FixedOffset'
State: closed

xref https://github.com/pandas-dev/pandas/pull/28914#issuecomment-541064775

@WillAyd  we _could_ use `from pytz import FixedOffset, ...` so that only a single `type: ignores` is required.


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

Comments:
Out of curiosity do you know how hard it would be to do this in typeshed? I *think* they pull in latest type shed just before the release gets cut and from the few PRs I've done type shed turnaround is a few days at most. Might be able to solve directly at source instead of here
This will be an issue with mypy 0.730, so a typeshed update won't fix straight away.
> Out of curiosity do you know how hard it would be to do this in typeshed?

i'll see what's required, submit a PR, then add the issue number here.

if mypy bumps on conda in the meantime we could just merge as is.
why don’t we just fix mypy at the current version?
Similar to will’s idea, is there a clean fix upstream in pytz?
> why don’t we just fix mypy at the current version?

brief discussion started here https://github.com/pandas-dev/pandas/issues/27568#issuecomment-528933381
> Similar to will’s idea, is there a clean fix upstream in pytz?

pytz is in typeshed, which ships with mypy. but missing FizedOffset.

types in typeshed are created by the community.


i'll close this. we should just pin mypy in ci.
add uint64 support for some libgroupby funcs
State: closed

cc @WillAyd I hope you agree the runtime_error thing here is easier to implement/review with fused type than it would be with tempita.

Labels:
name: Groupby
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
Can you add tests for these as well? Would be nice to have something parametrized in groupby
> Can you add tests for these as well? Would be nice to have something parametrized in groupby

I'll take a look and see if we have tests that get at these functions directly (as opposed to via df.groupby.whatever).  I can attest that these are hit by the python code for which this adds a comment.
Following this and #28934 one more obvious cleanup is to move all of groupby_helper into groupby.pyx, since we are no longer using the templating
@WillAyd it looks like we don't have any dedicated testing that gets directly a the functions in this file.
Any major concerns?  This is a blocker for getting at the `except Exception`s in/around _cython_agg_blocks
> I think you can add to the parametrization of this test here:

@WillAyd added uint64 to this parametrization.
I think a whatsnew for function support would be nice as well
> I think a whatsnew for function support would be nice as well

Only user-facing effect should be perf, which I haven't measured yet.  Would prefer to do measurements in one thorough pass after the last `except Exception` is removed around cython_agg_general (this plus two PRs)
thanks
ENH: Support for specifying col names for col_space
State: closed

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


Labels:
name: Enhancement
name: IO HTML
description: read_html, to_html, Styler.apply, Styler.applymap

Comments:
Hello @VijayantSoni! 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/io/formats/html.py`](https://github.com/pandas-dev/pandas/blob/aeacf155df253c2d2a02815f5840c66911526918/pandas/io/formats/html.py):

> [Line 127:89](https://github.com/pandas-dev/pandas/blob/aeacf155df253c2d2a02815f5840c66911526918/pandas/io/formats/html.py#L127): [E501](https://duckduckgo.com/?q=pep8%20E501) line too long (92 > 88 characters)


Can you add test(s) for the changed behavior
@VijayantSoni are you still working on this? Agreed with @simonjayhawkins this shouldn't be a new keyword. You can model `col_space` off of what `formatters` currently accepts
@WillAyd yeah, will work on changes.
@WillAyd @simonjayhawkins Hey guys, just to update, I won't be able to work on this for another 25-20 days. Please feel free to take it up. If this is still open after that time, I will continue.
> @WillAyd @simonjayhawkins Hey guys, just to update, I won't be able to work on this for another 25-20 days. Please feel free to take it up. If this is still open after that time, I will continue.

@VijayantSoni Thanks for looking into this. I'll close for now to clear the queue. can always re-open. ping when you wish to continue.
ENH: pd.MultiIndex.get_loc(np.nan)
State: closed

MultiIndex.get_loc could not find nan with values including missing
values as a input.

Background: In `MultiIndex`, missing value is denoted by -1 in codes and doesn't exist in `self.levels`

So, could not find NA value in `self.levels`.

Before PR xref #28783

- [x] closes #19132
- [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
name: MultiIndex

Comments:
Hello @proost! 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-01-07 13:28:41 UTC
@jbrockmendel @jreback 
`get_slice_bound`, `__contains__`, `slice_indexer`, `slice_locs` changes from this change. In short, these functions can index NA value. I think, this is better than before. Because As we treat NA value like a non-NA value, we can index more accurately when NA value is in 'MultiIndex'

If,
```
idx = MultiIndex.from_arrays([[np.nan, "a", "b"], ["c", "d", "e"]])
```

case1 : "get_slice_bound" 
```
idx.get_slice_bound(np.nan, side="left", kind="ix")
Before:   raise exception
After:  0
```

case2 : "slice_indexer"
```
idx.slice_indexer(start=np.nan, end="a")   
Before: raise exception
After: slice(0, 2, None)
```

case3: "slice_locs"
```
idx.slice_locs(start=np.nan, end=("a","d"))  
Before: raise exception
After: (0,2)
```

case4: `__contains__`
```
assert np.nan in idx
Before: raise exception
After: Not raise exception
```
problem is when this PR is merged,   if `__contains__` is true, but "isin" can be false. because "isin" not affect from this change. so if this PR is merged, need to change "isin" for a consistency. 




> problem is when this PR is merged, if __contains__ is true, but "isin" can be false. because "isin" not affect from this change. so if this PR is merged, need to change "isin" for a consistency.

can you show what you mean here?
@jreback 

```
idx = MultiIndex.from_arrays([[np.nan, "a", "b"], ["c", "d", "e"]])
assert np.nan in idx
```
not raise exception. But
```
idx.isin([np.nan],level=0)
array([False, False, False])
```
`__contains__` says NA value in level 0 index, But `isin` says doesn't. 

So  If this PR is merged, change `isin` for consistency might be good.

can you merge master and will look again
> So If this PR is merged, change isin for consistency might be good.

ok this PR looks good. can you add in support for .isin() in this PR? I think it would make sense to merge these simultaneously (e.g. you can also build on this one if that is easier)
@jreback 
Okay, I change `.isin` and add tests
thanks @proost very nice!
BUG: Avoid undefined behaviour when converting from float to timedelta
State: closed

Summation of timedelta series with NaTs in them result in undefined
behaviour because the final wrapping step of the summation ends up
converting the NaNs in the sum through a direct cast to int64.  This
cast is undefined for NaN and just happens to work on x86_64 because
of the way `cvttd2si` works.  On Aarch64, the corresponding `fcvtzs` sets
the result to 0 on undefined input.

This fix trivially sets the conversion target to m8 instead of i8 so
that numpy correctly casts from NaN to NaT.  Note that the fix in
numpy for the same is pending in PR numpy/numpy#14669 .

There is an existing test (test_sum_nanops_timedelta in
frame/test_analytics.py) that exercises this bug and has been verified
to have been fixed with this and the numpy patch.

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Timedelta
description: Timedelta data type

Comments:
> There is an existing test (test_sum_nanops_timedelta in
frame/test_analytics.py) that exercises this bug and has been verified
to have been fixed with this and the numpy patch.

is there a test that would fail currently on our CI and pass with this patch? or is this just platform specific?
cc @jbrockmendel 
> > There is an existing test (test_sum_nanops_timedelta in
> > frame/test_analytics.py) that exercises this bug and has been verified
> > to have been fixed with this and the numpy patch.
> 
> is there a test that would fail currently on our CI and pass with this patch? or is this just platform specific?

It looks like all of your CI tests are x86-only, so you won't see this issue.  If you run the testsuite on aarch64 though, you'll see `test_sum_nanops_timedelta` fail without this patch and pass with it.
> If you run the testsuite on aarch64 though

Do azure or travis offer such a thing?  (Or anyone else?)
> > If you run the testsuite on aarch64 though
> 
> Do azure or travis offer such a thing? (Or anyone else?)

I believe Travis does now, as of just 4 days ago:

https://blog.travis-ci.com/2019-10-07-multi-cpu-architecture-support
thanks @siddhesh 

would be interested in adding a travis build on arm? (can you open a separate issue to track).
TYPES: add types in core.util.hashing
State: closed

cc @simonjayhawkins 

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

Comments:
@simonjayhawkins changing np.uint64 to int caused mypy to complain.  any other suggestions?
Shouldn't the return type of `_combine_hash_arrays` be an ndarray? The annotation says `int` but mypy is complaining because on line 210 we are trying to index what `_combine_hash_arrays` returns
> Shouldn't the return type of _combine_hash_arrays be an ndarray

I think you're right, yes.  Will update.  I look forward to the day when annotations support cython-like `ndarray[uint64, ndim=1]` or something.
Closing for now to clear the queue, will troubleshoot locally.
TYPING: lockdown test modules passing mypy
State: closed

xref https://github.com/pandas-dev/pandas/pull/28904#discussion_r333740051 and https://github.com/pandas-dev/pandas/pull/28746#discussion_r330499267

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

Comments:
Are some of these easy to fix? Would be nice to keep the setup.cfg small if could do as precursors
maybe, not really looked in detail. and I personally won't be prioritising this over the main codebase.

it was so others, could add type hints to the tests and have them checked(well most of them)  see #28904

also if doing it on a per module basis, easier to confirm in review if the module is also removed from setup.cfg (process adopted for check_untyped_defs, seems to work well)

Sounds good. No major objection from me - @jreback any thoughts?
not at desktop atm. will give you a better idea of the workload tomorrow. i _think_ it was about 200 errors. but many duplicates. and some files _might_ only have a couple.

I _think_ if the extra noise in setup.cfg is an issue, we could consider using mypy.ini
didn’t we use mypy.ini as some point?

i guess that’s better 
Yea went from mypy.ini to setup.cfg in #27427 - I don't know if there are any real nuances to going back so would be OK with that
ok, yeah let's keep in the same
FYI xref #28910

now that this is on master, a mypy bump to 0.730 would break ci 

```
$ mypy pandas
pandas\tests\indexes\datetimes\test_tools.py:270: error: Module has no attribute "FixedOffset"
pandas\tests\indexes\datetimes\test_tools.py:275: error: Module has no attribute "FixedOffset"
pandas\tests\indexes\datetimes\test_tools.py:281: error: Module has no attribute "FixedOffset"
pandas\tests\indexes\datetimes\test_tools.py:282: error: Module has no attribute "FixedOffset"
pandas\tests\indexes\datetimes\test_tools.py:290: error: Module has no attribute "FixedOffset"
pandas\tests\indexes\datetimes\test_tools.py:292: error: Module has no attribute "FixedOffset"
pandas\tests\indexes\datetimes\test_tools.py:2082: error: Module has no attribute "FixedOffset"
pandas\tests\indexes\datetimes\test_tools.py:2087: error: Module has no attribute "FixedOffset"
Found 8 errors in 1 file (checked 805 source files)
```

PR coming soon. just parking here for reference.
CLN: simplify maybe_promote in float and complex cases
State: closed

Analogous to #28899 (orthogonal) for complex and float dtypes.  I'm pretty sure that after the current crop of maybe_promote PRs goes through we can do another round of consolidating code.

Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Clean

Comments:
thanks @jbrockmendel
Maintain Timezone Awareness with to_json and date_format="iso"
State: closed

- [X] closes #12997
- [X] tests added / passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry

This vendors updates from numpy that allow for tz-aware ISO date formatting. Note this does slightly change the behavior of UTC dates. Previously they would write out as `2013-01-01T05:00:00.000Z` but now are `2013-01-01T05:00:00.000+00:00`. Both are valid ISO 8601

There is a follow up that needs to be addressed with reading these dates

Labels:
name: IO JSON
description: read_json, to_json, json_normalize
name: Timezones
description: Timezone data dtype

Comments:
are these changes mostly copy/pasted from numpy?  IIRC we used to use copy/pasted versions for some of these but i edited them to remove unused or always-0 args, woops.
Closing for now as I haven't been actively working this and want to keep the queue down, but plan on coming back to it one day
TYPING: errors reported by mypy 0.730
State: closed

errors reported by mypy 0.730 on master.

```
pandas\compat\pickle_compat.py:71: error: Incompatible return type for "__new__" (returns "Series", but must return a subtype of "_LoadSparseSeries")
pandas\compat\pickle_compat.py:85: error: Incompatible return type for "__new__" (returns "DataFrame", but must return a subtype of "_LoadSparseFrame")
```

probably makes sense to apply these changes now to avoid ci breakage on mypy bump.


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

Comments:
thanks @simonjayhawkins
TYPING: fix type annotation for pandas.io.formats.format._binify
State: closed

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


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

Comments:
thanks
REF: simplify maybe_promote integer cases
State: closed

@jreback its a little absurd how much this can be simplified after all the trouble I went through to on the last round

Let's hope the float cases simplify as nicely (separate branch/PR)

Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Clean

Comments:
wow no good deed goes unpunished!
Yah, luckily the other cases turn out to simplify nicely too (#28913).  Really looking forward to being done with this
REF: maybe_promote refactor/cleanup
State: closed

There is some casting we currently do in many places in this function.  Instead this PR implements `_ensure_dtype_type` and calls it once at the end of the function.

Removes _check_lossless_cast which can be replaced with np.can_cast

Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Clean

Comments:
can you rebase
rebased+green
thanks
def item( was broken
State: closed

`pd.DataFrame({"a":[1,2,3]}).a.item() ` -- breaks! 
`pd.DataFrame({"a":[1,2,3]}).a.values.item(0)` --- this is actual expected behaviour

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:

Comments:
@avloss this has been the behaviour as long as it existed I think. Numpy supports `item()` without argument as well. It means that it only works for objects with length 1 and otherwise raise.  

It could make sense to actually pass through the argument, but since this method is deprecated, I don't think we should still make improvements to the method.
Yea I think this method needs to be removed altogether for 1.0 (ref #6581) - @avloss are you interested in cleaning that up?
As mentioned this is a deprecated method so I don't think we have an appeatite for this. In any case thanks for the PR! If you'd like to contribute would certainly take the actual removal of this method altogether
DOC: Update performance comparison section of io docs
State: closed

xref https://github.com/python-sprints/pandas-mentoring/issues/163


Labels:
name: Docs

Comments:
Hi Marc @datapythonista 
Hi Marc @datapythonista .
Kindly check this PR?
I restored the indentation for a moment, to have an easier diff (but can undo that in the end, as a 4-space indent is easier to work with)
lgtm. @jorisvandenbossche 
@WuraolaOyewusi can you fix up the merge conflict? I think we can merge this in once complete
@WillAyd 
Done
Great thanks @WuraolaOyewusi keep em coming
DOC: Fixed PR06 error in pandas.Categorical.from_codes
State: closed

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


Labels:
name: Docs

Comments:
Already contained within #28807
DOC: Fixed PR06 error in pandas.io.formats.style.Styler.set_table_attributes
State: closed

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


Labels:
name: Docs

Comments:
Thanks @roshni-b
REF: use fused types for groupby_helper
State: closed

There will be some nice cleanups we can do after we bump cython to 0.30 (which hasnt come out yet).

Also I think there is some na-checking code that we can share between the various fused-types functions after this.

Labels:
name: Groupby
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
>  I'd still prefer to keep as is and just add the uint template generation as required until 0.30 gets released

Since we're probably going to wait for a while after 0.30 is released before bumping ourselves, that is longer than I'd prefer.

As for using uint template generation in the interim, I did that before deciding that I am much more likely to screw up using tempita than I am using fused types.
thanks @jbrockmendel 

we cannot wait for deps, who knows when they land
DOC: Fixed PR06 errors in pandas.api.extensions.ExtensionArray
State: closed

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


Labels:
name: Docs

Comments:
Great thanks @roshni-b
Remove ix, part 1
State: closed

- [ ] xref #15113 and #26438 

First part of removal od ``NDFrame.ix``.  There's still alot of tests for this, so I'll do this removal in 2-3 steps. This PR removes pandas/tests/indexing/test_ix.py and tests for ix in a few more places.

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Deprecate
description: Functionality to remove in pandas

Comments:
Just seen #6581, where it says ix is to be removed in 1.2. So this is perhaps too early?
Anything deprecated prior to 0.25 is good to remove for 1.0, so no issue there.

There is however #27620 out there as well
ix is good to remove now
I’ll just close this down, given that #27620 is already on this...
TST: Fix maybe_promote floating non-boxed tests
State: closed

Like #28864, this involves changing the underlying function.  Some of this can be de-duplicated once these are in.

I think that this is the last of the xfails for non-boxed test cases.  Really looking forward to having this done with,.

Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
thanks