BrandonFors/pandas-github-issues huggingface dataset:
BUG: Wrong dtype when resetting a multiindex with missing values. (#1…
State: closed

…9602 and #24206)

- Fixed the bad path in reset_index where the dtype was ignored if there are only
missing values.

- Simplified the structure of _maybe_casted_values in reset_index by
using as much as possible the take method of ExtensionArray's

- [ ] closes #19602
- [ ] closes #24206
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
Two possible improvements that i noticed while working on this :

1) The multiindex path of _maybe_casted_values is essentially a take(…, allow_fill=True) function that works on both ExtensionArrays and ndarrays. Since it's the type of the _values attribute, it might be a good idea to extract it outside of reset_index if it's used in other places. (I thought that it likely is but I didn't really know.)

2) When refactoring _maybe_casted_values in reset_index, I tried removing the cast at the start of the function (frame.py line 4607) : 
```        
            if not isinstance(index, (PeriodIndex, DatetimeIndex)):
                if values.dtype == np.object_:
                    values = lib.maybe_convert_objects(values)`
```

It was introduced a very long time ago (#440) to try and infer the type of indexes with dtype object. Removing it breaks one test that relies on the fact that an index with integer underlying data and object dtype will be correctly casted to an Int64Index. I didn't have enough knowledge of the code base to know if inferring the type of object indexes is still the intended behavior, but if it's not these lines could be removed.
@jreback for info: I have made all the modifications you requested up to now and on my side its ready. If you need further changes I can try to handle them before next friday, but after that I will be away for 3 weeks and unable to update the pull request.
@ldmnt can you merge master and we'll take a look.
can you merge master
Sorry I haven't found time to do that recently. I saw your message, I will try to do the merge and think about Tom's question this week.
Thanks for the PR but I think this has gone stale. Closing to clean up the queue for now, but certainly ping if you'd like to pick back up
BUG: Need 'windows-1252' encoding for locale names.
State: closed

https://github.com/pandas-dev/pandas/issues/24760
https://github.com/pandas-dev/pandas/issues/23638
There are some special characters encoded with 'window-1252' in lists
created by 'locale -a'.  The two know locales with this problem are
Norwegian 'bokmål', and 'français'.

- [x] closes #24760  Already closed but previous resolution was a work-around
- [x] closed #23638  Already closed without resolution
- [ ] tests added / passed
- [ ] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Unicode
description: Unicode strings
name: Linux
description: Linux OS

Comments:
Is there any chance that using `subprocess.run` instead of `subprocess.check_output` helps here at all? We only support >=Py35 so could switch from the older API if it helps simplify
> Is there any chance that using `subprocess.run` instead of `subprocess.check_output` helps here at all? We only support >=Py35 so could switch from the older API if it helps simplify

Neither `subprocess.run` nor `subprocess.check_output` should change or set the output encoding.  The encoding should only depend on what `locale -a` prints.  Why 'windows-1252' instead of 'utf-8'?  I have no idea.
Also be sure to run `black pandas` on your local branch before pushing (that is the current CI failure)
@timcera can you add a test and merge master?
this looks ok actually, @timcera can you add a note to 1.0; if you can do a test great, but not a big deal otherwise.
@timcera can you rebase?  I think the CI failure may be unrelated
this is prob ok, can you merge master and add a note to 1.0.0, ping on green.
Have to ask - what does "note to 1.0.0, ping on green" mean?

On Sun, Oct 6, 2019 at 7:55 PM Jeff Reback <notifications@github.com> wrote:

> this is prob ok, can you merge master and add a note to 1.0.0, ping on
> green.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/27368?email_source=notifications&email_token=AACIRNV26PFREBCT4MGYTPLQNJ3IPA5CNFSM4ICRV4OKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAOXK7I#issuecomment-538801533>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AACIRNXVKDLVQX7FUDQULCTQNJ3IPANCNFSM4ICRV4OA>
> .
>

Can you add a note to `doc/source/whatsnew/v1.0.0.rst` describing the change and then notify when CI is green after pushing up the change?
All checks passed.
thanks @timcera
BUG: Fix timezone-related indexing and plotting bugs
State: closed

- [x] closes #15552
- [x] closes #20441
- [x] closes #17173
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


1 timezone plotting bug, 1 indexing bug with tz data, 1 timezone data regression test

Labels:
name: Bug
name: Visualization
description: plotting
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Timezones
description: Timezone data dtype

Comments:
thanks @mroeschke
Add small docs about xlsxwriter in user guide
State: closed

- [x ] closes #27356
- [ ] tests added / passed
  - Not needed.  Just a doc change
- [ ] passes `black pandas`
  - Not needed.  Just a doc change
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
  - Not needed.  Just a doc change
- [ ] whatsnew entry
  - Not needed.  Just a doc change

Labels:
name: Docs
name: IO Excel
description: read_excel, to_excel

Comments:
Great thanks @Dr-Irv
TST: add test for series list indexing with missing values
State: closed

- [ ] closes #11428
- [ ] 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: Indexing
description: Related to indexing on series/frames, not to indexes themselves

Comments:
lgtm. merge on green.
adds non regression test for GH27358
State: closed

- [X] closes #27358
- [X] tests added / passed
- [ ] 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:
looks ok @jreback :)
thanks!
TST: add test for multiindex partial indexing both axis
State: closed

- [ ] closes #12660
- [ ] 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: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: MultiIndex

Comments:
thanks @simonjayhawkins
Dispatch Index ops to Series
State: closed

Getting ever closer to a single implementation.

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

Comments:
Looks reasonable. What's the ideal final state? For future mico-optimization, I could imagine an implementation that does index-specific things (e.g. deferring for `op(Index, Series)`) and then just `box(op(array1, array2))`. It's a bit unfortunate that we have to pay the Series constructor performance tax on each op.
> What's the ideal final state?

- Contingent on 2D EA happening, the ops are defined directly on PandasArray and either Index._data is a PandasArray or calls PandasArray(self._data) in the op instead of Series(self)
- Otherwise, it'll take some closure-rearrangement, but I think we can get a version of those ops that operates directly on ndarray.
- Regardless: a single implementation so we get internal consistency for free (not-yet-conformant: RangeIndex, IntegerArray, SparseArray)
thanks @jbrockmendel
CLN/REF: indexing typing, prune unreachable branches
State: closed


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

Comments:
still would like to move to a unified pandas/core/indexing/..... structure sooner rather than later
ok to merge, though if you want to split the setting routine I think might be helpful here.
> though if you want to split the setting routine I think might be helpful here.

will do in next round.  lots of balls (branches) in the air
> > though if you want to split the setting routine I think might be helpful here.
> 
> will do in next round. lots of balls (branches) in the air

k
thanks
CLN: Collapse private ._take implementation into the public take method
State: closed

- [x] closes #27174
- [ ] tests added / passed
- [X] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Getting rid the `_take` private method that seems to be no longer needed.


Labels:
name: Clean

Comments:
thanks @endenis
CLN: Split test_window.py further
State: closed

closes #19228
xref #26807
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] passes `black pandas`

Just splitted code into files by classes. No changes made to code

cc @jreback 

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

Comments:
Not sure we need this (probably should have closed first issue after last PR). Any interest in parametrizing instead?
Opened as per @jreback's [comment](https://github.com/pandas-dev/pandas/pull/27275#issuecomment-510564495).
My mistake! Though would still take parametrization (separate PR is fine) if you are up for it
@WillAyd thanks for suggestion noted that
If this is a pure split, then I would be in favor of merging this.
thanks @ihsansecer
Fix _can_hold_element for datetimelike blocks
State: closed

I've got a bunch of branches going that touch this, easier to just get it right once.

Labels:
name: Internals
description: Related to non-user accessible pandas implementation
name: Clean

Comments:
> so this basically unified DatetimeBlock and DateimtTZBlock for _can_hold_element?

Yes.  ATM DatetimeTZBlock._can_hold_element is ExtensionBlock._can_hold_element, which always returns True
lgtm. ping on green.
The CI fail is weird.  It’s getting a warning about elementwise comparison but it’s a scalar being compared.
ping
gentle pint; this is a semi-blocker
> will try this out in a dedicated branch
 
Thanks. +1 on the changes here, pending the resolution of `is_integer` / `is_integer_strict` (can we think of a different suffix than `_strict`? That doesn't really convey that (just) time delta are excluded). 
> Does the whole world break if you just add the exclusion of timedelta64 to is_integer

Nope, just opened #27401 

I think the thing to do here is to revert the last commit to get rid of is_integer_strict and move forward with the more verbose variant, then trim that if/when #27401 goes in.
ping
thanks @jbrockmendel
CLN: revisit build warnings in cython templates
State: closed

- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Retrying dropped portions from #27157, which amend cython templates, to deal with special-cases based on type. In this Pr, compiler warnings about  `x==x` being useless in the case of ints, but not floats (since `nan!=nan`).

In https://github.com/pandas-dev/pandas/pull/27157#discussion_r299103166 these were rejected because there wasn't agreement about how this should done and a separate discussion was requested. Annoyingly, these changes agree with what already exists in these files (in multiple places):

https://github.com/pandas-dev/pandas/blob/355e322b5ed4a31c9e1cab9652510e85a54f23a3/pandas/_libs/groupby_helper.pxi.in#L212-L216

But the file is already inconsistent in how these type checks are done:

https://github.com/pandas-dev/pandas/blob/355e322b5ed4a31c9e1cab9652510e85a54f23a3/pandas/_libs/groupby_helper.pxi.in#L391

Making a decision here also required to get rid of the warnings mentioned in https://github.com/pandas-dev/pandas/issues/27169, in that case fused types which mix signed/unsigned trigger warning when the code later includes comparisons with a signed quantity. So there too, type checks in the template are the obvious fix.

cc @WillAyd, @jreback 


Labels:
name: Clean

Comments:
I'm not really sure this is an improvement as it just adds more layers to the templating where we already have a weird mix between templates and fused types. Is there a way to just do this all using fused types perhaps?
actually i think these changes are ok, though can you have a look and see how consistent we are with these idioms across cython? is there a way to centralize these templates (easily)? e.g. have a is_null type of template
> is there a way to centralize these templates (easily)?

maybe, I'll try it. But then, what would to do with the signed/unsigned compares in #27157?
ok this is positive progress towards reducing warnings.
Accept empty dataframes in DataFrame.to_parquet
State: closed

Fixes #27339

I wrote two tests because `fastparquet` adds a name to the index when it deserializes.

- [x] closes #xxxx
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Compat
description: pandas objects compatability with Numpy or Python functions
name: IO Parquet
description: parquet, feather

Comments:
Thanks @jreback. Should this go in the whatsnew for 0.25.0 or 0.25.1?
> Thanks @jreback. Should this go in the whatsnew for 0.25.0 or 0.25.1?

you can put in 0.25.0
lgtm. @CJStadler ping on green.
It's green @jreback. Thanks!
thanks!
ENH: add NDFrame.select_str
State: closed

- [x] closes #26642
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Adds a method ``select_str`` to supplement ``select_dtypes``. This PR is intended to be followed by a PR that deprecate ``NDFrame.filter``.

I miss the most having a string focused axis-filtering method, and is ``NDFrame.filter`` a bit unfocused  and badly named IMO. I think usage of parameter ``items`` in ``NDFrame.filter`` can just be replaced by recommending ``NDFrame.__getitem__`` and parameter ``like`` is less useful than ``startswith`` and ``endswith`` (is less precise). So this is not a 100 % replacement for ``NDFrame.filter``, but intends to make something that is more useful (IMO, of course).

This is not fully tested and need a whatsnew etc. but I would like to get some response before I go further.


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

Comments:
Hello @topper-123! 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-07-11 20:38:22 UTC
Closing in favor of #27363
CLN: remove Hypothesis warning during test
State: closed


```plain
hypothesis\_settings.py:193: HypothesisDeprecationWarning: The timeout setting can 
safely be removed with no effect.
```

Starting with 4.0.0 released in Jan, the timeout setting does [nothing](https://github.com/HypothesisWorks/hypothesis/commit/2b9506b7120c2f0593539c1e5d9b526d7ec1377f).

https://hypothesis.readthedocs.io/en/latest/settings.html#hypothesis.settings.timeout
```plain
    The timeout setting has been deprecated and no longer does anything.

    default value: not_set

    The timeout setting can safely be removed with no effect.
```

Bump dependency from `>=3.8.2` and remove setting.


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

Comments:
The bump here just suppresses the warning right? Would also want to add a note to min dependencies section in 0.25 whatsnew
> The bump here just suppresses the warning right?

Yeah, and it seems to be gone from CI. Don't expect any breaking due to upgrade.

> you also need to update any ci/deps where we explicity set the hypothesis versions

thanks.
Turns out hypothesis 4.0.0 dropped python3.5 support. What do you want to do?
Does just removing the timeout without bumping get us to the same place?

Sent from my iPhone

> On Jul 11, 2019, at 12:36 PM, pilkibun <notifications@github.com> wrote:
> 
> Turns out hypothesis 4.0.0 dropped python3.5 support. What do you want to do?
> 
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or mute the thread.

> Does just removing the timeout without bumping get us to the same place?

For the CI? yeah, in that even without bumping 'hypothesis>=3.58.2' CI installs the current version. a user who hasn't updated deps in 8 months may run into some timeout issues. Sounds ok.
this is ok, ping on green.
@jreback green.
ENH: Use IntergerArray to avoid forced conversion from integer to float
State: closed

- [ ] closes #xxxx
- [x] tests added / passed
- [ ] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

This is a new attempt to solve issue #16918 after an attempt with PR #27073

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

Comments:
Is there a way to make this work without adding a keyword to the DataFrame constructor? That approach seems off to me

Also be sure to add tests with this - its not entirely clear that this does anything as is
@WillAyd @jreback If the original implementation of using DataFrame to do the expansion of `list` of `dict` to 2d array and the type determination for columns is to be maintained then it seems to me that there is no way to circumvent propagating a new key in the DataFrame to make it backwards compatible. Is it more desirable to pull out the expansion and type determination logic from DataFrame and have some code duplications?

I will add tests later, but I want to know what is the more accepted methods here first.
@jiangyue12392 we are likely going to do the inference for expanding a int dtype with nulls to integer-na (rather than float) at some point, but currently this is NOT the default. We need some more infrastructure first (see the linked issue).
@jreback Ok, I will wait till the infrastructure is in place after #27283 is sorted out
@jiangyue12392 actually part of this PR could partially satisfy that issue
@jreback I suppose the part where the change in `lib.pyx` serves the purpose of distinguishing an `integer_na`. However, on that issue, both @TomAugspurger and @jorisvandenbossche express their concern to keep behaviour consistent with the past where the default datatype is `float64`. I am not sure how users can choose between the new `Int64` behaviour or the old `float64` for backward compatibility
so what you do is make the return string integer-na (this is a small api change)
there are a small number of places then that would have to interpret this type (as float) for casting purposes

that’s it
@jiangyue12392 can you merge master
> @jiangyue12392 can you merge master

Sorry, I have been busy with other stuff recently, will merge as soon as I get my hands free
@jiangyue12392 if you can merge master; I think parts of this patch we would want to merge.
> @jiangyue12392 if you can merge master; I think parts of this patch we would want to merge.

Master merged. Sorry for the delay.
> if you just do the internal change in maybe_convert_objects and add a test for that we can get this in. pls merge master as wel.

I have made the required changes and added the test.
> small comment, pls rebase and ping on green.

Green now.
thanks @jiangyue12392 nice patch. This should enable us to selectively convert to nullable integers with the user facing routines.
CLN: requested follow-ups
State: closed

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


Labels:
name: Internals
description: Related to non-user accessible pandas implementation
name: Clean

Comments:
lgtm. ping on green.
ping
thanks!
BUG: fix+test assigning invalid NAT-like to DTA/TDA/PA
State: closed

Broken off from #27311 for cleaner scope and to troubleshoot platform-specific failures.  Related but orthogonal to #27323.

Labels:
name: Datetime
description: Datetime data dtype
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: Timedelta
description: Timedelta data type
name: Period
description: Period data type

Comments:
comments addressed i think
BUG: reindex with empty inputs
State: closed

…seless in that case anyway

- [x] closes #27315
- [ ] tests added / passed
- [x] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


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

Comments:
find the reindex tests; we need to run all combinations of args for an empty
Will do when I find some more time this week... New to the python world and this project so I need a few more moments to get familiar with the testing.
@jreback, could the tests be a while loop on a dict list using **kwargs on the method or are you looking for explicit individual tests for each arg combination? The former makes sense to me for efficiency, but being relatively new to python projects I'm not sure whether that overly obfuscates failure messages. Also I noticed the entry point on reindex tests are not on the class where I discovered the issue or even in the stack trace of those errors, and the class that I changed is further down inheritance of the tested class in there. Should we write tests for higher level DataFrame.reindex or should we go for the lowest public granularity of where (assuming private methods are not test targets like other languages) the changes are at Index.reindex? I can see reasons for either decision based on skimming through the class inheritance patterns.

Sorry if these are remedial questions, just want to make my limited time focused and productive while reducing review cycle tedium for you where my knowledge gaps are. I'll make decisions once I get into it either way if there's no preference, but some foresight would save some time if there are preferences.

Also sorry for the delay, too many commitments on my plate at the moment. Next step for this is to get a list of args for the test which is simple but tedious enough to conflict my schedule. If anyone wants to assist I can probably fit resolving any test failures into my schedule easier. If nobody claims that testing task I will wrap this up as soon as I can.
@ajspera you should be able to parametrize the various combinations of parameters. If you search for parametrize in the tests you should see quite a few examples to work off of
@ajspera is this still active?
closing as stale; this does fix the issue but needs tests. if you want to continue, pls merge master and ping to re-open.