BrandonFors/pandas-github-issues huggingface dataset:
ENH: add way to check for sparse data (#26706)
State: closed

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


Labels:

Comments:
> it seems that other `is_...` ones we have are attributes (at least the ones in Series, not sure if we have public ones in other places), so we should probably be consistent with that.

Actually, looking a bit further, we have also `is_..` methods/attributes on Index, and there `is_all_dates`, `is_unique`, `is_monotonic` and some others are attributes, but `is_boolean/categorical/floating/integer/numeric/...` are methods ... So in that case maybe rather keep as a method?
-1 as already indicated as this is unecessary api bloat 

we don’t have is_float or is_integer as properties (they already exist as functions)
Jeff, did you see this is on the `.sparse` accessor? (we don't have a float or integer accessor, so any comparison isn't really correct) So it does not add to the top-level DataFrame namespace.

On the issue (https://github.com/pandas-dev/pandas/issues/26706) you mentioned `select_dtypes` as what should be used, but @TomAugspurger explained how that is not an alternative. Please respond to the specifics if you do not agree with that.
@jorisvandenbossche your original issue shows this works just fine with .apply
so -1 on any expansion here; even in the accessor this doesn’t make any sense (even less sense)

we already have multiple ways of selectimg dtypes and i don’t see any justification 
I think this makes sense to add.

Slight preference for it being a property, rather than a method call, but either works.
Should I update the PR to make it a property? 
Probably wait till we come to an agreement.

My argument for property is that this is essentially statically known
(though there is a bit of computation).

On Wed, Sep 11, 2019 at 2:26 AM SdgJlbl <notifications@github.com> wrote:

> Should I update the PR to make it a property?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/28314?email_source=notifications&email_token=AAKAOIXP6ASEAA6MTG4AJH3QJCMSFA5CNFSM4IUIVVAKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6NRNRI#issuecomment-530257605>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAKAOIUIW5H2AOFCUZHVJNLQJCMSFANCNFSM4IUIVVAA>
> .
>

> My argument for property is that this is essentially statically known
(though there is a bit of computation).

That's a good argument (the `Index.is_boolean()/is_numeric()/...` which are methods check the inferred type, so with potentially a lot of computation). I am fine with going for a property then.
I don't believe there is a consensus on this change so closing for now to clear queue; can pick reopen if one is reached later
Added a way to check if a numeric type cell is NaN
State: closed

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


Labels:
name: IO Excel
description: read_excel, to_excel
name: Needs Discussion
description: Requires discussion from core team before further action

Comments:
is there an issue describing why the current behavior is wrong?  can you add a test?
I am -1 on converting NA values to 0 - is there an open issue about this?
@antonini thanks for the interest in submitting a PR! I don't know if this is a change we actually want to make though. If you feel strongly about it I'd suggest opening an issue first and can be discussed there
BUG: make tz_localize operate on values rather than categories
State: closed

- [x] closes #27952
- [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: Categorical
description: Categorical Data Type

Comments:
this also needs a bit more general testing, try to add a few more accessor functions that you are testing.
Thanks @jbrockmendel and @jreback for your comments.

I've moved the location of the tests to indexes/test_category and have removed the `to_list` call. However, I am using `.__array__()`. Is this also expensive?

I can't see how to keep `data` as a Series with CategoricalDtype, because then I'm blocked by #28448 .
@TomAugspurger @jbrockmendel @jreback thanks for your reviews. Now that #28762 has been closed, I've been able to push a solution to this which doesn't involve `np.asarray` and IMO is a marginally cleaner.

Is this something you're still interested in? Should the return type (and thus the documentation) be changed as well?

If not, no worries, thanks for the time you've already dedicated to helping me, feel free to close it :)
sure this still makes sense
@MarcoGorelli can you rebase and we'll take a fresh look and try to get this through
Sure, done
@jreback Sure, here you go
thanks @MarcoGorelli nice patch!
fix Rolling for multi-index and reversed index.
State: closed

Fix Rolling operation for level of multi-index and descending time index (that is monotonic, but decreasing).

- [x] closes #19248
- [x] closes #15584
- [x] tests passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


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

Comments:
Does this close a particular issue? Also tests are the most critical part of any PR so would be great if you can add
> Does this close a particular issue? 

I found those two just now:
https://github.com/pandas-dev/pandas/issues/19248
https://github.com/pandas-dev/pandas/issues/15584

> Also tests are the most critical part of any PR so would be great if you can add

Will do.


Hello @leftys! 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-19 20:11:05 UTC
@WillAyd I added tests and updated whatsnew to reference the two issues.
can you update the top section with the issue references (use a closes #....)
@jreback Fixed according to review comments.
Thanks @leftys - very nice PR!
BUG: Merge on CategoricalIndex fails if left_index=True & right_index=True, but not if on={index} #28189
State: closed

- [x] closes #28189 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

This modification resolves the error in issue #28189 but still not working as expected. It seems that there is a bug related with left-join, as you can see in issues  #28220 and #28243. 

I'm making this pull request for the #28189 in case you want to resolve this bug separately from the left join problem. On the other hand, I can work on this and help is welcome. 





Labels:
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Categorical
description: Categorical Data Type

Comments:
@hugoecarl can you merge master and we'll have another look.
By merging master you mean updating our branch with upstream?
> By merging master you mean updating our branch with upstream?

yes

```git merge upstream/master```
@hugoecarl @gabriellm1 is this still active? Can you resolve merge conflicts?
Closing as stale but ping if you'd like to pick back up
Fix inconsistent casting to bool
State: closed

Series logical ops (`|`, `&`, `^`) have a bunch of inconsistencies.  This partially addresses one of them, and does so in a way that makes the diff for the next one much more manageable than it would be without this.

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

Comments:
Does this improve my commit, does it supersede it or is it unrelated?
@Unprocessable what commit are you referring to?
This one: #28251
Unrelated.
thanks @jbrockmendel
BUG: Series[timdelta64].var() should _not_ work
State: closed

- [x] closes #18880
- [x] 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:
Haven't deeply read the issue this closes but is this the agreed upon route? This would still allow `std` to work just not `var` right? That seems kind of strange to me though I'm not super familiar in this area
> This would still allow `std` to work just not `var` right?

Correct, `std` is still allowed while `var` is not.

> That seems kind of strange to me though I'm not super familiar in this area

Some discussion of why this is the correct behavior in #18880.  Recapping that:

Suppose `X` is some quantity with a unit `u` (e.g. meters, or kilograms, or seconds, or in this case nanoseconds).  The `mean(X)` has that same unit `u`.  `Var(X)` has unit `u^2`.

In the case where `u` is `nanoseconds`, we do not have any way of representing `u^2`, so cannot return a correct answer for `Var(X)`.

`std(X)` is defined as `sqrt(Var(X))`, which has unit `sqrt(u^2)`, which is just `u`, which we can handle correctly.
So IIUC it's mostly a technical limitation in expression `u^2` since precision is in nanosecond?
> So IIUC it's mostly a technical limitation in expression u^2 since precision is in nanosecond?

"technical limitation" yes, "precision is in nanosecond" not clear.  It's not about the "nano" precision being squared, its about the "timedelta" being squared.
Don't want to go too far off the rails but does the arithmetic here work generally? I guess I could see an end user constructing something as follows:

```python
>>> ser = pd.Series([pd.Timedelta("1S"), pd.Timedelta("3S")])

# std should be 1S but isn't?
>>> ser.std()
Timedelta('0 days 00:00:01.414213')
# var should be 1S maybe as well?
>>> std.var()
2e+18
```

I suppose for scope of this PR maybe disallowing the second is fine, but shouldn't the former be disallowed then as well? May be better than buggy if I've got my math right until someone wants to come along and fully implement

the `std` result not being 1 second is a result of the `ddof` (denominator(?) degree of freedom) correction that is dividing by `len(ser) - 1` instead of `len(ser)`.  So if you do `ser.std(ddof=0)` you'll get the result you expect.
Ah good catch...

> Can you also add a whatsnew that this explicitly raises a TypeError?

Sure
conflict, plus a comment above
gentle ping.  more reductions to implement after this
I don't think my comments above were addressed. the code change is duplicative, if you can find a way to combine with the existing routines.
> I don't think my comments above were addressed. the code change is duplicative, if you can find a way to combine with the existing routines.

I replied to your comment suggesting that nanstd should go through nanvar: that is not a viable option.
thanks
DOC: Fix PR06 errors in Series docstrings (#28253)
State: closed

First bit of work on #28253, fixes PR06 validation errors in Series docstrings.
Mostly just converting boolean -> bool, string -> str, integer -> ; re-worded a little where necessary.

- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Ran validate_docstrings.py and confirmed all PR06 issues for Series are removed, and that no additional validation errors are added by this change.


Labels:
name: Docs

Comments:
Great thanks @danielplawrence !
BUG: datetime64 - Timestamp incorrectly raising TypeError
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: Datetime
description: Datetime data dtype

Comments:
Travis has been done for a few hours, no idea why it’s stuck on yellow
lgtm. can you rebase and push again.
rebased+green
Thanks @jbrockmendel
DOC: Fix typo in min_rows / max_rows option example
State: closed

 - Fix typo in example
 - Regenerated output also seems to fix a bad output in the next line (out 32 in current online documentation) where df is truncated, when it shouldn't by according to the code 


Labels:
name: Docs

Comments:
Also, when looking at the [current online documentation](https://pandas.pydata.org/pandas-docs/stable/user_guide/options.html), more things seem to be off. The example about `display.expand_frame_repr`does not show any difference between the two settings. Rebuilding the page seems to fix this, my local version looks good. 
@datapythonista this looks fine to me, but I don't know this area that well.

Great thanks @tobycheese
BUG: passing DataFrame to make_block silently raises
State: closed

ATM in the `except NotImplementedError:` branch of `_cython_agg_blocks`, we take a difference approach to the operation.  if that approach succeeds, we end up passing a DataFrame to `make_block` on L189, which will raise `ValueError`.  As a result, we'll end up falling back to python-space for the entire operation, which presumably entails a performance hit.

This fixes the incorrect passing of DataFrame, but the fix is kind of kludgy.  Suggestions welcome on how to improve it.

Labels:

Comments:
Hmm so you are saying this improves mixed block performance where some can be cythonized right?

This adds a little bit of logic so would be good to see that measured somehow to understand if it’s worth it
> Hmm so you are saying this improves mixed block performance where some can be cythonized right?

I'm having a hard time cooking up an example, but roughly what I have in mind is a case with 

1) a bunch of non-EA blocks and then 
2) one EA block such that 
  a) NotImplementedError is raised at L164 (L160 in PR) and 
  b) the fallback on L179 (L180 in PR) goes through OK, so that
  c) in master, we get a ValueError when passing `result` to `block.make_block` on L180

In such a case we would end up falling back to the non-cython implementations for _all_ blocks instead of just the one relevant EA block.
Closing to clear the queue.  Will revisit.
DEV: remove seed isort config
State: closed

This was causing issues for me locally. Anyone else?

It took a while to run, and didn't seem to give the same
output as others (depends on something peculiar to my environment)
which doesn't seem to be great for a pre-commit hook.

Closes https://github.com/pandas-dev/pandas/issues/28236

Labels:
name: CI
description: Continuous Integration

Comments:
OK by me but cc @sbalk and @basnijholt for insight
I added it because it solved a particular problem. I didn’t anticipate that it would cause any problems.  I’m also fine with deleting it.
BUG: Timestamp+int should raise NullFrequencyError, not ValueError
State: closed

Last prerequisite before we can fix #28080.

Labels:
name: Bug
name: Frequency
description: DateOffsets

Comments:
What would a release note for this look like? IIUC, this is backwards compatible since catching a ValueError will still catch this NullFrequencyError? But we should still have a note describing the change so that users could tighten up the caught exception if they desire.
> What would a release note for this look like?

This is a good question.  I'm hesitant to advertise the change because before long we'll be enforcing the deprecation, at which point this should raise TypeError.
I think OK to just say something like `... now raises a NullFrequencyError` without worrying too much about the details of a future deprecation
lgtm. maybe rebase and push again (as the CI is hanging)
rebased+green
Looks like some merge conflicts - can you fix up and repush?
rebased+green
thanks @jbrockmendel
BUG: Make sure correct values are passed to Rolling._on when axis=1
State: closed

- [x] closes #28192
- [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: Window
description: rolling, ewma, expanding

Comments:
lgtm. @mroeschke think we need more testing for this?
Hello @MarcoGorelli! 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-09-04 16:32:05 UTC
Thanks @MarcoGorelli!
BUG: pd.to_datetime fixed (#28238)
State: closed

- [ ] closes #28238
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Fix for KeyError when trying to convert a series with unsorted float index to a DateTime 



@AntonioAndraues
@Vinigl


Labels:

Comments:
Here's some prints showing the fix. I just changed what @mroeschke suggested in https://github.com/pandas-dev/pandas/issues/28238#issuecomment-526772229

![bug_before](https://user-images.githubusercontent.com/13368721/64176002-e65c3500-ce32-11e9-9593-a978e5cb31b7.png)

![bug_after](https://user-images.githubusercontent.com/13368721/64176001-e65c3500-ce32-11e9-8463-272f81eb3476.png)

I think @mroeschke was suggesting that change as a workaround in user's code, not as a change to pandas. Rather, we'll want to fix the bug that only surfaces with `cache=True`.
closing as per comments
Contribute to issue #15580
State: closed

- [ ] closes #15580
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

I'm new at contributing on Pandas and any project at all, and saw this generic "fix docs" issue. I made some very small changes that i think it helps users to understand better the `groupBy` usage. Please, feel free to judge my changes and request modifications. Thanks!

Labels:

Comments:
Thanks for the PR! I don't think we want to merge any of these changes though so going to close this one.

We definitely appreciate contributions so if you are interested in the area I would suggest checking the issue tracker for items labeled `good first issue` and/or `docs` - those are a great place to start contributing
re-implement #27959
State: closed

Previous version broke because a different branch changed the behavior of when extract_array is called.

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

Comments:
Any idea why this wasn't caught by CI the first time around?
> Any idea why this wasn't caught by CI the first time around?

Between the time when the #27959 went up and the time when it was merged, a different PR changed the expected inputs to `dispatch_to_extension_op`.  Previously `extract_array` was called inside `dispatch_to_extension_op`, but that was changed to expect the inputs to already be extracted.  27959 was calling using the older convention, which led to RecursionError.
Fix to_json Memory Tests
State: closed

Inspired by @mroeschke I noticed the JSON tests weren't actually measuring anything because you need to return something for the `mem_` tests which these weren't. In any case probably better served as `peakmem_`


Labels:
name: IO JSON
description: read_json, to_json, json_normalize
name: Benchmark
description: Performance (ASV) benchmarks

Comments:
Can you re-push following #28258
All fixed up @jbrockmendel - thanks for the heads up
thanks @WillAyd
BUG: CategoricalIndex allowed reindexing duplicate sources
State: closed

For consistency with normal indexes, `CategoricalIndex.reindex` should allow targets to be duplicated but not the sources. However, currently in allows the source to be duplicated but not the targets, which is exactly the wrong behaviour.

Most of the work here is fixing the tests, which in many cases explicitly check for the incorrect behaviour.

Fixes #25459 

(I can't run the full testsuite on my machine but the relevant categorical tests do seem to pass. Hoping that the GitHub CI infrastructure will pick up any other failures.)

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

Comments:
Hello @batterseapower! 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-11 09:47:23 UTC
Had to fix a couple of incidental Pandas bugs that were surfaced by the main fix.

1) `pd.Index([1, 1, 0, 2, 2]).union(pd.Index([0, 2, -1]))` would fail with an error about duplicates in the index. We now return `pd.Index([-1, ,0 1, 1, 2, 2])` (or `pd.Index([1, 1, 0, 2, 2, -1])` if `sort=False`) which is more consistent with the behaviour of `Index.intersection`

2) `pd.Index(['A', 'B']).get_indexer_non_unique(pd.Index([0]))` would fail, complaining with a `TypeError` about `'<' not supported between instances of 'str' and 'int'`. This is caused by over-aggressive use of `searchsorted`, solved by falling back to linear search
Please let me know if this needs more work..
lgtm. cc @jbrockmendel @jschendel if any comments.
LGTM
thanks @batterseapower
Datetime mergeasof tolerance
State: closed

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


Labels:
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Timedelta
description: Timedelta data type

Comments:
@ianzur - this is great! Mind updating as per @jreback comments looks like this is almost there 
Thanks for the reminder! I got distracted with school.

On Mon, Sep 23, 2019 at 10:41 AM alimcmaster1 <notifications@github.com>
wrote:

> @ianzur <https://github.com/ianzur> - this is great! Mind updating as per
> @jreback <https://github.com/jreback> comments looks like this is almost
> there
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/28241?email_source=notifications&email_token=AICYMWMMTMSUQ24OPG4YNQDQLDPSNA5CNFSM4ISTXQ22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7LJPSQ#issuecomment-534157258>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AICYMWKARA6KQ5YMIA3HVCDQLDPSNANCNFSM4ISTXQ2Q>
> .
>

Does this need a test? Can we now get rid of the xfail for test_tolerance in test_merge_asof? 
@alimcmaster1 There is already a test in place. The test is not set to xfail.

https://github.com/pandas-dev/pandas/blob/8de8b081a77a484cd0e4e73dd88abe9de99bfd75/pandas/tests/reshape/merge/test_merge_asof.py#L593-L605
@ianzur - LGTM now thanks! Yep see that now (wasn’t showing up earlier)
Thanks @ianzur.
fix DatetimeIndex.tz_localize examples docstring
State: closed

[DatetimeIndex.tz_localize](https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.DatetimeIndex.tz_localize.html#pandas.DatetimeIndex.tz_localize) doc string has improperly formatted examples near the bottom. Trivial fix, so didn't think it needed an issue or whatsnew entry, but happy to add if needed. Now looks like:

<img width="765" alt="Screen Shot 2019-08-30 at 12 55 16 PM" src="https://user-images.githubusercontent.com/4383303/64048047-73e31080-cb25-11e9-9266-ef721bc2600b.png">


- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Docs

Comments:
Thanks!
REGR: Fix to_csv with IntervalIndex
State: closed

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


Labels:
name: Regression
description: Functionality that used to work in a prior pandas version
name: IO CSV
description: read_csv, to_csv
name: Interval
description: Interval data type

Comments:
Thanks!