BrandonFors/pandas-github-issues huggingface dataset:
CLN: F-strings
State: closed

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


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

Comments:
Great thanks @dinasv
ENH/TST: Allow more keywords to ensure_clean
State: closed

These keywords will be passed through to `tempfile` constructor functions.

Follow-up:

https://github.com/pandas-dev/pandas/pull/30771#discussion_r363684666

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

Comments:
@WillAyd @jreback : Any further thoughts on this?
I would still vote for explicit arguments if anything but certainly open to input of others
@pandas-dev/pandas-core : Thoughts?  There seems to be some impasse regarding whether we should surface explicit arguments.  I'm all for being explicit, but I also want them to make sense for both `mkstemp` and `TemporaryFile` and not just one.

As we have it, `filename` is used consistently for both constructor functions.
this is fine as its only for testing, thanks @gfyoung 

however, I think it would be interesting to separate ensure_clean to 2 functions, e.g. ensure_clean_dir and ensure_clean_file; then you *could* have named keywords.

not sure how much work that would be but might be a good followup .
DOC: Added documentation for ImportError's
State: closed

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


Labels:
name: Docs

Comments:
I've made the changes you've mentioned! While building on my system, I didn't face any issues, but while building it here, I'm unable to find the reason for the build failure. Could you help me out @datapythonista ?
@datapythonista Can you check if this can be merged? All the checks have passed!
Thanks!
Thanks @gonemad97
CLN: remove geopandas compat code
State: closed

CC @jorisvandenbossche its safe to remove this now right?

Labels:
name: Compat
description: pandas objects compatability with Numpy or Python functions
name: Clean

Comments:
@jbrockmendel this is meant for 1.1 (so only released in a couple of months)? 
Then it's probably fine to remove yes, otherwise I would prefer to keep it a bit longer.
1.1 works for me
Thanks!
BUG: EAs should not be hashable
State: closed

This turns out to be a blocker for some cleanup in index.pyx

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

Comments:
LGTM - do we need a `whatsnew` ?
Do we actually want to commit to this? Just wonder if this would conflict with some of the work I think @TomAugspurger did before to try and create a non-mutable dataframe
yes mutable things are not hashable
we do allow hash_pandas_object to work though
This change seems fine. Can you say more about the motivation though? As I can imagine an immutable array that is hasable, so we mange shouldn’t rely on unhashabilty. 
> Can you say more about the motivation though? As I can imagine an immutable array that is hasable, so we mange shouldn’t rely on unhashabilty.

I'm working on indexing code and want to simplify a check

```
cdef inline bint is_definitely_invalid_key(object val):
    if isinstance(val, tuple):
        try:
            hash(val)
        except TypeError:
            return True

    # we have a _data, means we are a NDFrame
    return (isinstance(val, slice) or util.is_array(val)
            or isinstance(val, list) or hasattr(val, '_data'))

    cpdef get_loc(self, object val):
        cdef:
            Py_ssize_t loc

        if is_definitely_invalid_key(val):
            raise TypeError(f"'{val}' is an invalid key")
    [...]
```

to just check for hashability.  Doing that breaks a couple of tests because EAs are currently hashable
> to just check for hashability. Doing that breaks a couple of tests because EAs are currently hashable

When an immutable, 3rd-party EA comes along that's hashable, will that break again?
> When an immutable, 3rd-party EA comes along that's hashable, will that break again?

If that EA gets passed as a `key` to `index._engine.(get_loc|get_value|contains)` then it may give `(KeyError|KeyError|False)` instead of raising a `TypeError`
thanks @jbrockmendel
BUG: pickle files left behind by tm.round_trip_pickle
State: closed


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

Comments:
worth backporting

merge on green
green
Thanks @jbrockmendel Is it worth moving check_for_file_leaks into a higher level conftest?
> Is it worth moving check_for_file_leaks into a higher level conftest?

Different kind of file leak
BUG: SystemError in df.sum
State: closed

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


Labels:
name: Datetime
description: Datetime data dtype
name: Regression
description: Functionality that used to work in a prior pandas version
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations

Comments:
Hmm that's strange. Is there an upstream issue for this?
@scoder any idea if this is an issue in cython vs further upstream?
Planning to merge this later today.
No issues in merging but if this is the resolution I guess the question is do we need to worry about this in other places where Cython / f-strings are used?
Something to keep an eye on / debug for sure. I'm not really sure where to start on that (the linked issue has a bit more information). Disallowing fstrings in Cython code is certainly one option.
Thanks @jbrockmendel!
Implement C Level Timedelta ISO Function; fix JSON usage
State: closed

- [X] closes #28256
- [X] tests added / passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry

similar to @cbertinato great work in #28595 but done deeper in the C extension

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

Comments:
Current benchmarks:

```sh
       before           after         ratio
     [447a3b00]       [4a94f159]
     <master>         <json-timedelta>
+         152±3ms         209±10ms     1.38  io.json.ToJSONISO.time_iso_format('values')
+         168±3ms         213±10ms     1.27  io.json.ToJSONISO.time_iso_format('records')
+         188±4ms         235±10ms     1.25  io.json.ToJSONISO.time_iso_format('split')
+         195±4ms         238±10ms     1.22  io.json.ToJSONISO.time_iso_format('columns')
-        103±10ms         79.2±6ms     0.77  io.json.ToJSON.time_to_json('values', 'df_td_int_ts')
-        137±20ms         95.9±1ms     0.70  io.json.ToJSON.time_to_json('values', 'df_int_float_str')
```

So the time_iso_format checks are taking a little bit longer but note that they were broken before, i.e. on master you get this:

```python
>>> pd.Series([pd.Timedelta(days=1)], index=[pd.Timedelta(days=1)]).to_json(date_format="iso")
'{"P1DT0H0M0S":"1970-01-02T00:00:00.000Z"}'
```
Where we are writing as dates and not durations.

I think just a matter of the timedelta -> iso function not being as fast (using sprintf instead of totally hand-coded) but maybe OK for now
pls rebase
LGTM cc @jreback
needs rebase, other good to go i think
completely closes #28256. ?

can you check the perf again.
Yea closes; added some more test coverage as well.

Here is an updated benchmark:

```sh
       before           after         ratio
     [402c5cdd]       [ef08ad6c]
     <json-test-cleanup~1^2>       <json-timedelta>
+        143±20ms         225±10ms     1.57  io.json.ToJSONISO.time_iso_format('values')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE DECREASED.
```
this is slower?
Slower but correct
> Slower but correct

worth the tradeoff every time.
thanks, correct wins every time.
BUG: raise on non-hashable in __contains__
State: closed

By checking before we get to the `Engine.__contains__` call, we can avoid a redundant call to `hash`

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Index
description: Related to the Index class or subclasses

Comments:
mypy didnt like my attempt at annotating, so reverted
> mypy didnt like my attempt at annotating, so reverted

is this because \_\_contains__ is a special method and this code is unsafe. Is this not also a breaking change?
restored annotations.  turns out the trouble was that `RangeIndex.__contains__` had a non-matching annotation
> restored annotations. turns out the trouble was that `RangeIndex.__contains__` had a non-matching annotation

I think mypy should only complain if the class is a subclass of collections.abc.Container
even so, in typeshed, classes that want to restrict the type of key add a type:ignore.

so rather than Any, can Hashable be used?
> so rather than Any, can Hashable be used?

When I use `Optional[Hashable]` mypy gives:

```
pandas/core/indexes/numeric.py:236: error: No overload variant of "int" matches argument type "Optional[Hashable]"
pandas/core/indexes/numeric.py:236: note: Possible overload variant:
pandas/core/indexes/numeric.py:236: note:     def int(self, x: Union[str, bytes, SupportsInt] = ...) -> int
pandas/core/indexes/numeric.py:236: note:     <1 more non-matching overload not shown>
pandas/core/indexes/numeric.py:488: error: Argument 1 to "len" has incompatible type "Optional[Hashable]"; expected "Sized"
pandas/core/indexes/numeric.py:488: error: Item "Hashable" of "Optional[Hashable]" has no attribute "item"
pandas/core/indexes/numeric.py:488: error: Item "None" of "Optional[Hashable]" has no attribute "item"
pandas/core/indexes/numeric.py:490: error: Argument 1 to "len" has incompatible type "Optional[Hashable]"; expected "Sized"
Found 5 errors in 1 file (checked 846 source files)
```
Yea I guess might be tough to get that to work with mypy since we are catching the `TypeError` at runtime, though passing a non-hashable item to that would currently raise an error at the `hash()` call that is uncaught, so I think mypy is highlighting an actual bug in a roundabout way
> looks fine. can you add assert hash(key) in the cython code?

A big part of the motivation here is to avoid duplicated hash calls
> > looks fine. can you add assert hash(key) in the cython code?
> 
> A big part of the motivation here is to avoid duplicated hash calls

do asserts not get stripped? isn't that the point? (its not a big deal as long as you have a comment, but the asserts are to provide developer guarantees)

also needs a rebase
thanks @jbrockmendel
TYP: typing annotations
State: closed

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


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

Comments:
Thanks @MomIsBestFriend
CLN: simplify Float64Index.__contains__
State: closed

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


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

Comments:
Thanks @jbrockmendel
DOC: Add pandas_path to the accesor list in the documentation
State: closed

Added the [`pandas_path`](https://github.com/drivendataorg/pandas-path/) package that I just released to the list of third-party accessor extensions.

As a bonus, added a Description column for the existing libraries.

Labels:
name: Docs

Comments:
@datapythonista Cool, I added a longer description to the "Data cleaning" section, which seemed the most relevant. That make sense?
Thanks @pjbull
TYP: offsets
State: closed


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

Comments:
@TomAugspurger are things like this mergeable during the RC or are they just cluttering the queue for the next couple of weeks?

Side-note: running the tests has started leaving a bunch of .pickle files behind.  Is that just me?
I've noticed the stray pickle files. Not sure what test is leaving them.

IMO, these kind of PRs can be merged to master, but don't need to be
backported. So they'll be included in 1.1.

On Fri, Jan 10, 2020 at 1:21 PM jbrockmendel <notifications@github.com>
wrote:

> @TomAugspurger <https://github.com/TomAugspurger> are things like this
> mergeable during the RC or are they just cluttering the queue for the next
> couple of weeks?
>
> Side-note: running the tests has started leaving a bunch of .pickle files
> behind. Is that just me?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/30897?email_source=notifications&email_token=AAKAOIRGEHZBT6FMWUQ6KW3Q5DDCZA5CNFSM4KFLKP62YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIU6JIQ#issuecomment-573170850>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAKAOIQP5GCF52NWGA7G2SDQ5DDCZANCNFSM4KFLKP6Q>
> .
>

Thanks @jbrockmendel
[BUG] Series.diff was always setting the dtype to object
State: closed

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


Labels:

Comments:
@jreback @jbrockmendel @WillAyd thanks for your quick reviews, I've updated the PR accordingly
> Isn't the correct fix to to dispatch `diff` to the EA, rather than trying to fix the type after the fact? Or can we perhaps write a version of `diff` in terms of `take` and `__sub__`?

this is actually a good idea

would be simple to write as a generic EA
method
@TomAugspurger @jreback I don't understand.

Currently, we have
```
        result = algorithms.diff(com.values_from_object(self), periods)
```
In the case of
```
pd.Series([1, 2, 3], dtype="Int16").diff()
```
then 
```
com.values_from_object(self)
```
is already converting the values to Object before even reaching `algorithms.diff`:
```
>>> com.values_from_object(self)
array([1, 2, 3], dtype=object)
```

So should `com.values_from_object` be changed?

Anyway, I realise that there's a somewhat tight deadline to get v1.0.0 out, and I'm happy to keep tackling this issue, but if you think it is (currently) beyond my capabilities, I'm happy for someone else to take over :)
> So should com.values_from_object be changed?

We would need to not call that, at least for EAs, but probably for all dtypes, before passing it off. I can probably take a look tomorrow.

We have a week or two for 1.0.0 final.
@TomAugspurger OK thanks - I *might* have figured out what you were suggesting anyway, have pushed another commit
See https://github.com/pandas-dev/pandas/pull/31025 for an alternative which allows EAs to define how the operation should be done.
I think we'll proceed with https://github.com/pandas-dev/pandas/pull/31025. Apologies for the duplicated effort @MarcoGorelli.
It's OK, it was a good learning experience to attempt this and then read through your PR!
WEB: Link from the website to the docs
State: closed

The navigation of the web was implemented assuming the docs will render with the same layout. Since this is not finally the case, and the website and the docs are finally independent, I guess it makes more sense to simply link to the home of the docs.

Otherwise, we'd need to rethink those links, since the last one is broken, and the rest are inconsistent with the navigation of the docs.

Closes https://github.com/pandas-dev/pandas/issues/31657


Labels:
name: Web
description: pandas website

Comments:
I actually find navigating to these from the dropdown on the main page pretty nice (save Older Versions, which doesn't work) - this does away with those right?
I'm ok with the dropdown too, but it requires deciding which items we want on it, and find agreement on wherever we want the dropdown or not (I think @jorisvandenbossche didn't want it).

Given that we're releasing soon, and one of the links is broken, I thought it was better to go for this simple option. And we can add the dropdown back in a follow up. 
Yeah, in many cases, I don't like much dropdowns (but for sure sometimes there are good use cases for it). 

In this specific case, I find personally that it doesn't add value. For example, right now, to go to the user guide, it takes 2 clicks (docs dropdown + user guide). If it's not a dropdown, but just a link to the docs, this also takes 2 clicks, so not much difference IMO (although your mouse will need to move a bit more between both clicks). But in addition, you are directly on the doc page, and can also click on anything that is not in the dropdown.
For the versions, we could already add a simple page (to the web/ ?) that just lists them. I was working on a feature in the sphinx theme to add a version dropdown (yes, a dropdown! ;)), but didn't find the time yet to finish that
Closing this, since it's more controversial than expected. Opened an issue for the discussions: #31061
Personally, I would rather merge this for now (or otherwise at least remove the broken versions link)
Objections to merge this as short-term fix? (since people start to report the broken link)
Merging this so we at least don't have a broken link for the v1.0.1 update (the website still is updated manually right now when we release, it's not yet automated from github)

We can still further discuss which links we want to include here (eg the versions are right now also not yet available on the docs itself)
DOC: Move import conventions from wiki to docs #30808
State: closed

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


Labels:
name: Docs

Comments:
@datapythonista I was looking into testing files and there all imports are absolute but previously (according to wiki) explicit relative imports were encouraged. Are absolute imports encouraged in test files also?
Lots of people contribute to pandas, so if we don't enforce the policies with validation in the CI, you'll find any possible kind of practice in the code base. I think the preferred option is using absolute imports, so for now, if you can move that policy to the docs, that's a first step. Then, we may considering if its worth detecting relative imports, fixing them, and validating in the CI that they are not reintroduced. Feel free to open an issue to discuss that after this is closed. 
@datapythonista, I have updated the docs. Let me know if there should be any changes.
Thanks @souvik3333 
Thanks both, I deleted the wiki page.
[DOC] set klass correctly for series and dataframe set_axis
State: closed

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


Labels:
name: Docs

Comments:
Thanks @MarcoGorelli for the PR. will look soon.
sorry @MarcoGorelli struggling to get doc build locally and now AFK for the best part of the day.
@simonjayhawkins no hurry :)
@MarcoGorelli should probably also change `See Also` to have %(klass)s substitution and use substitution for `An object of same type as caller` otherwise lgtm.
@simonjayhawkins sure, done - thanks for your review!
Thanks @WillAyd for the review.

Sure, here's the output of
`python make.py clean`
`python make.py --single pandas.DataFrame.set_axis`
`python make.py --single pandas.Series.set_axis`


![image](https://user-images.githubusercontent.com/33491632/72601530-81026f80-390d-11ea-9970-af5c62306bd7.png)
![image](https://user-images.githubusercontent.com/33491632/72603370-cb392000-3910-11ea-8f5a-c14f3bcd246a.png)

Thanks @MarcoGorelli
raise more specific error if dict is appended to frame wit…
State: closed

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


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

Comments:
Thanks @MarcoGorelli
ENH Avoid redundant CSS in Styler.render
State: closed

Where multiple styled cells have the same CSS, only make one CSS declaration. This can reduce the output size substantially.

- [x] ~~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: IO HTML
description: read_html, to_html, Styler.apply, Styler.applymap

Comments:
Any interest in this?
this seems ok to me, basically writing the 'same' style, but de-duplicated so its much smaller? @TomAugspurger 
> basically writing the 'same' style, but de-duplicated so its much smaller?

Yes

thanks @jnothman
DOC: add note about many removals in pandas 1.0
State: closed

cc @TomAugspurger 
Adding a note about the recommended way to upgrade to pandas 1.0, if that sounds OK

Labels:
name: Docs

Comments:
(this has always been the case before with our rolling deprecations, but I think it can be stressed now more because 1) we removed really a lot and 2) we also removed things only deprecated in 0.25)
Looks good, can you fix the merge conflict?
@TomAugspurger : Merge conflict in this PR was introduced by me, so I resolved it myself.

(an aside: surprised that there was a merge conflict.  The diff in no way overlaps)
DOC: Encourage use of pre-commit in the docs
State: closed

Previously, we stated it as merely optional.

xref https://github.com/pandas-dev/pandas/pull/30773, https://github.com/pandas-dev/pandas/pull/30814


Labels:
name: Docs

Comments:
lgtm. Having not really use pre-commit that much does it also play nicely with VSCode? Probably the route most first-time contributors come through so just want to make sure it doesn't add complications there
https://github.com/magicmark/pre-commit-vscode

https://marketplace.visualstudio.com/items?itemName=MarkLarah.pre-commit-vscode
>  does it also play nicely with VSCode? 

What possible complications do you think of? 
The pre-commit hook runs a few checks. Either you have already enable flake8 and black in vscode, and then the pre-commit won't complain, or you do not have auto-formatting enabled, and the pre-commit hook will reformat for black / print the errors for flake8 (this what I do, works fine).

I suppose if you have black auto-formatting enabled in vscode, you could run into problems with different configurations? 

> I suppose if you have black auto-formatting enabled in vscode, you could run into problems with different configurations?

You can disable plugins by workspace in VSCode, so I wouldn't consider this an issue.

https://code.visualstudio.com/docs/editor/extension-gallery#_disable-an-extension
BUG: ExtensionArray._formatter does not need to handle nulls
State: closed

See https://github.com/pandas-dev/pandas/pull/30821/files#r364917242

Currently, I don't think this is really specified whether the `_formatter` of ExtensionArray should be able to handle NAs/nulls or not, but in practice, it didn't need to do that. And eg the geopandas one is also implemented right now that it would fail if passed a NA (https://github.com/geopandas/geopandas/blob/add5fe9139883fa54d14fa28426478619948349c/geopandas/array.py#L1026-L1047)

This is certainly something that can be discussed how to specify this, but for now I think it is best to preserve the existing behaviour.

Labels:

Comments:
cc @TomAugspurger