BrandonFors/pandas-github-issues huggingface dataset:
PERF: lazify pytz seqToRE call, trims 35ms from import
State: closed

Shaves on the order of 5% off of import time.

Unrelated docstring fixups, improve typing on parse_timezone_directive and _calc_julian_from_V.

The important thing here is setting the "Z" key dynamically in `__getitem__` instead of in `__init__` (since an instance is created in the module namespace at importt)

Labels:
name: Performance
description: Memory or execution speed performance
name: Timezones
description: Timezone data dtype

Comments:
How does the __init__ impact import time? Are we instantiating these at import?
> How does the init impact import time? Are we instantiating these at import?

Yes, there is an instance of this in the module namespace.
lgtm to me
Thanks @jbrockmendel 
@WillAyd IIRC you made the top-level Panel import lazy in py37.  have you looked at e.g. https://pypi.org/project/modutil/ for doing this in more places?  top-level imports from pandas.io.api look like the next place we can trim
Haven't looked a lot at lazy imports, so if you have concrete proposals certainly put them out there.

Just keep in mind as one downside to lazy imports I think you would lose type checking against those libraries, since that import I don't think can be resolved statically
PERF: trim import time ~5%
State: closed

by avoiding/lazifying expensive stdlib imports.

Exact timings are tough because repeated runs of `python3 -X importtime -c "import pandas as pd"` are really high variance, but my general read is that this trims about 35 ms out of about 650 ms.


Labels:
name: Performance
description: Memory or execution speed performance

Comments:
Hmm so if we only expect a 5% improvement in import time I am actually -1 on this change. I don't think its worth adding runtime logic for a small optimization
> Can you add these modules to the blacklist in ci/code_checks.sh?

Looks like it checks for e.g. "urllib" as opposed to "urllib.request".  Since we also import urllib.parse, we can't exclude that.  Excluding "http" works though.
> so if we only expect a 5% improvement in import time

Some context on that number:

- #28228 gets another 5% or so
- numpy master (but not 1.17) makes numpy.testing import lazy, trimming about 15% from their import time, so about 3% of ours
- IIRC before long we'll stop importing matplotlib at import-time, which could trim our time by up to 20% (based on current timing of pandas.plotting._matplotlib import)

The 5% we get here becomes a bigger deal as those get whittled down
trimming import time is always good; sure it sometimes makes the code a bit more complex but +1 here
I'm ok with this, especially on the aggregate benefits, but suspect it will be brittle, as someone could in the future (unaware) undo this optimization, so we only get left with the complexity, and no benefit. Maybe add tests for the content of sys.modules in a seperate CI run?
Does the code check added not catch that?
Ah nevermind - I see that is only for the http module not urllib
Im also thinking more general, imports in imported modules etc.
We could modify the check. I don't know why I originally wrote it to just check the package.
edited code check
Wouldn't it make sense to have a whitelist instead of a blacklist? A blacklist will allow a lot of things that we might not really want when importing pandas.
Let's leave the idea of a whitelist changeover for a separate discussion.  Other than that, I think everything has been addressed here.
A merge of master should fix CI.
can you rebase
Rebased +green
Thanks.
Don't fail when plotting Series/DataFrame with no row
State: closed

When Series or DataFrames was empty (no cells) after removing columns
without a suitable type (.select_dtypes), pandas was throwing a "no
numeric data to plot" exception.

This can happen:

1) either because there is no column with a suitable type;

2) or if there is no row.

Raising an exception in the first case makes sense but we should
probably avoid throwing an exception in the second case.

- [X] closes #27758
- [x] tests added / passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Visualization
description: plotting

Comments:
Can you add a test for this?
Hello @randomstuff! 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-08-30 21:04:52 UTC
Yes, I was actually wondering how I should properly test this (and how the plot tests are done). I have added very basic tests.
Thanks @randomstuff!
Which exact pandas version was this added in?
PERF: data = np.nan to speed up empty dataframe creation 
State: closed

- [x] closes #28188 

- Short summary
If we have enough information to know the shape of the final
dataframe, we set the value of data to np.nan if not set.

- Longer explanation
If I understand correctly the `__init__` of DataFrame, with this change 
when the first if evaluates True then all following ifs evaluate False 
until we enter the else in [frame.py:459](https://github.com/pandas-dev/pandas/blob/03b3c8fc82b3a18a3ddcad1b3b26d601467fc74c/pandas/core/frame.py#L459), and we are done because now the dataframe is initialized with a ndarray.
With the former behavior `data` defaults always to None,
hence the output dataframe has all dtypes object and its creation
is slow.
With the proposed code the output dataframe has all dtypes float and is
faster to create than before, but only when columns and index are given.

I don't know how to loose the current restriction of `dtype=None`. It would be nice to have a function that verifies that a type is compatible with nan.


Labels:
name: Performance
description: Memory or execution speed performance

Comments:
first thing we look for a is a test
> 
> 
> first thing we look for a is a test

ofc! But I wanted to see also the results of CI to check what I'm breaking ;)
Hi @astyonax - thanks for the contribution. It looks like this might be an incomplete change however so going to close for the time being to keep our PR queue down.

Best advice I can give is to run the tests locally and make sure the change doesn't cause any regressions before pushing to CI. We have performance benchmarks you can run thereafter to measure improvements.

More info can be found in the contributing guide so be sure to give that a look:

https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#contributing-to-pandas
What kind of tests are needed on this one?
Fix read of py27 pytables tz attribute, gh#26443
State: closed

When created by python 2.7, the "tz" attribute will be created with CSET
H5T_CSET_ASCII instead of H5T_CSET_UTF8, therefore it is read as bytes
when string is expected.

- [ X] closes #26443
- [ X] tests added / passed
- [ X] passes `black pandas`
- [ X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ X] whatsnew entry


Labels:
name: IO HDF5
description: read_hdf, HDFStore

Comments:
Thanks for the PR but not sure about this as it makes it murky as to what aspects of Py27 we still support. Without this its very clear to say nothing, so I'm maybe -1 on this as I don't think we want to start down the path of offering any Py27 legacy support going forward.

Let's see what others think
1.0 (I don't think it was a 0.25.x regression).
REGR: <th> tags for notebook display closes #28204
State: closed

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


Labels:
name: Regression
description: Functionality that used to work in a prior pandas version
name: IO HTML
description: read_html, to_html, Styler.apply, Styler.applymap

Comments:
not added whatsnew. target 0.25.2?
Was it a regression in 0.25 or earlier?
haven't checked thoroughly but i think the bug was introduced in  0.25.1 #27991
With 0.25.0

```python
In [2]: df = pd.DataFrame({"a": 1}, index=pd.MultiIndex.from_product([['a'], [2, 3]]))

In [3]: print(df._repr_html_())
```

```html
<div>
<style scoped>
    .dataframe tbody tr th:only-of-type {
        vertical-align: middle;
    }

    .dataframe tbody tr th {
        vertical-align: top;
    }

    .dataframe thead th {
        text-align: right;
    }
</style>
<table border="1" class="dataframe">
  <thead>
    <tr style="text-align: right;">
      <th></th>
      <th></th>
      <th>a</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th rowspan="2" valign="top">a</th>
      <th>2</th>
      <td>1</td>
    </tr>
    <tr>
      <th>3</th>
      <td>1</td>
    </tr>
  </tbody>
</table>
</div>
```

and with 0.25.1

```html
In [3]: print(df._repr_html_())
<div>
<style scoped>
    .dataframe tbody tr th:only-of-type {
        vertical-align: middle;
    }

    .dataframe tbody tr th {
        vertical-align: top;
    }

    .dataframe thead th {
        text-align: right;
    }
</style>
<table border="1" class="dataframe">
  <thead>
    <tr style="text-align: right;">
      <th></th>
      <th></th>
      <th>a</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <td rowspan="2" valign="top">a</td>
      <td>2</td>
      <td>1</td>
    </tr>
    <tr>
      <td>3</td>
      <td>1</td>
    </tr>
  </tbody>
</table>
</div>
```

So it seems to have been a 0.25.1 regression (so target for 0.25.2)
thanks @TomAugspurger . will do.
BUG: Fix issue with apply on empty DataFrame
State: closed

Fixes a bug where the return value for certain functions was hard-coded as `np.nan` when operating on an empty `DataFrame`.

Before
```python
>>> import numpy as np
>>> import pandas as pd
>>> df = pd.DataFrame(columns=["a", "b", "c"])
>>> df.apply(np.sum)
a   NaN
b   NaN
c   NaN
dtype: float64
>>> df.apply(np.prod)
a   NaN
b   NaN
c   NaN
dtype: float64
```

After
```python
>>> import numpy as np
>>> import pandas as pd
>>> df = pd.DataFrame(columns=["a", "b", "c"])
>>> df.apply(np.sum)
a    0.0
b    0.0
c    0.0
dtype: float64
>>> df.apply(np.prod)
a    1.0
b    1.0
c    1.0
dtype: float64
```

**Edit**: Closes #28202 and closes #21959 after https://github.com/pandas-dev/pandas/pull/28213/commits/cb68153f4b6fca19440ec6b79a0d1128c002ec11.  The issue was that the arguments of `self.f` were already unpacked here:
https://github.com/pandas-dev/pandas/blob/cb68153f4b6fca19440ec6b79a0d1128c002ec11/pandas/core/apply.py#L112
and then we tried to do this again inside `apply_empty_result` which was raising an error and causing the unusual output.

Labels:
name: Apply
description: Apply, Aggregate, Transform, Map

Comments:
Hello @dsaxton! 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-12 13:19:32 UTC
needs a test
> needs a test

@jbrockmendel Added a few tests that appear to be failing on master.  There are some existing tests failing on this branch that I still need to work through.
> Also closes #28202

Does this close any other issues?

Can you add a release note in 1.0.0.rst?
> > Also closes #28202
> 
> Does this close any other issues?
> 
> Can you add a release note in 1.0.0.rst?

I believe it also closes #21959, which was still an issue at least as of 0.25.1 (not sure about master)
> Was the discussion on passing through args & kwargs resolved?

If I turn off the `try` / `catching` completely to try to surface any other issues, then adding back the argument unpacking only causes `test_nunique_empty` to fail among the tests in `test_apply.py` (three failed tests instead of the two mentioned above), so I would say we shouldn't be passing them.
Sounds good. I think we're good to merge then?
> Sounds good. I think we're good to merge then?

I'd say so unless @jreback can foresee any other potential problems
OK. Let's give it a few more days in case.
lgtm but need to edit the top of the PR to add closes on the other issue number 
> lgtm but need to edit the top of the PR to add closes on the other issue number

Done, thanks for the review!
Thanks @dsaxton
CLN: avoid catching Exception in _choose_path
State: closed

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


Labels:
name: Groupby
name: Clean

Comments:
If you are deep diving on this any chance you can add a docstring for func to help future readers?
lgtm. @WillAyd ?
Thanks @jbrockmendel
0.24.x | MyPy for Python2 Compatability - Patch "misplaced type annotation"
State: closed

This PR fixes misplaced annotations error, after checking with **MyPy**.

**What was fixed?**

For Python 2 you need place mypy annotations `# type: ...` before docstrings.

Labels:

Comments:
Thanks but no longer support Python 2 and don’t merge back to the 0.24 branch
Update frame.py
State: closed

Instead of just having lists in the examples, other "Index or array-like" types should be use likes tuples especially if they are immutable.

- [ ] closes #xxxx
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Docs

Comments:
Actually using list is clearer; a tuple can be ambigous, and might in some curcumstances signal a desire to insert the actual tuple as a single value. Using a list will always refer to a desire to insert the values inside the list.

Closing this issue. Thanks for the effort.
0.24.x 
State: closed

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


Labels:

Comments:
Hi there! We typically only accepts PRs against master, so if you have something you'd like to contribute please target that. Typically preferred if PRs reference an issue on GitHub as well.
Solving GL01,GL02 in pandas.Interval and a few mentioned in the comments
State: closed

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


Labels:
name: Docs

Comments:
Thanks!
DOC: Fix docs on merging categoricals.
State: closed

Fixes #28166.

Updated docs about merging categorical to reflect current behavior.

Labels:
name: Docs

Comments:
I've just noticed there is another section on [Concatenation](https://pandas.pydata.org/pandas-docs/stable/user_guide/categorical.html#concatenation), as opposed to [Merging](https://pandas.pydata.org/pandas-docs/stable/user_guide/categorical.html#merging), in the categorical docs. These docs describe different behavior, but still don't quite fit with what actually happens. Having two sections on this seems redundant to me, could these be collapsed to a single section in this PR?
I would be OK with that. It's what we seem to have set up in the 10 minutes to pandas:

https://pandas.pydata.org/pandas-docs/stable/getting_started/10min.html#merge
@ivirshup can you merge master and we will look again
@jreback, I've merged master (albeit a couple weeks ago now) and consolidated the sections on concatenation and merging.
@ivirshup can you address comment on table deletion? I think this is close
lgtm @jreback or @TomAugspurger if you want to look
Won't have time nearterm. Go for it when you're ready.

On Fri, Nov 8, 2019 at 10:10 AM William Ayd <notifications@github.com>
wrote:

> lgtm @jreback <https://github.com/jreback> or @TomAugspurger
> <https://github.com/TomAugspurger> if you want to look
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/28185?email_source=notifications&email_token=AAKAOISRC3ADR3PICYQNXEDQSWFO3A5CNFSM4IQ5YBY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDSSLZY#issuecomment-551888359>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAKAOIT2WRDSND3RKKNOTCDQSWFO3ANCNFSM4IQ5YBYQ>
> .
>

Thanks @ivirshup
Example for adding a calculated column in SQL and Pandas
State: closed

- [ ] closes https://github.com/pandas-dev/pandas/issues/28162
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Docs

Comments:
Thanks!
DOC: Add missing public plotting functions to the docs
State: closed

Some plotting public functions are missing from the documentation.

As proposed in #28177 I think those should be moved out of pandas or deprecated, but I guess it's worth having them documented as the rest of similar functions until that happens.



Labels:
name: Docs
name: Visualization
description: plotting

Comments:
Looks like the docstrings for these functions don't pass the validator - any chance you can clean those up as part of this?
Thanks for the head ups @WillAyd, didn't see it was failing.

Fixed the errors that we validate for now, I think it's worth to wait until a decision is made on #28177 before working in detail with those docstrings.
Thanks!
BUG: fix custom xticks in bar() plotting function
State: closed

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

Comments:
@GiuPassarelli can you merge master and fix conflict? I think just one outstanding comment here otherwise right?
Thanks @GiuPassarelli!
ENH: implement TimedeltaArray/TimedeltaIIndex sum, median, std
State: closed

The signatures match those in PandasArray.

- [x] closes #25335

Labels:
name: Datetime
description: Datetime data dtype
name: Algos
description: Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff

Comments:
If that’s it then just could just add return None at the end of that func

Sent from my iPhone

> On Aug 27, 2019, at 7:41 PM, jbrockmendel <notifications@github.com> wrote:
> 
> @jbrockmendel commented on this pull request.
> 
> In pandas/core/arrays/timedeltas.py:
> 
> > @@ -384,6 +386,49 @@ def astype(self, dtype, copy=True):
>              return self
>          return dtl.DatetimeLikeArrayMixin.astype(self, dtype, copy=copy)
>  
> +    def sum(
> $ mypy pandas/core/arrays/timedeltas.py
> pandas/core/indexes/base.py:257: error: Missing return statement
> 
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or mute the thread.

> If that’s it then just could just add return None at the end of that func

Looks like that func is `Index.__new__` and the complaint is about the branch that calls `cls._scalar_data_error(data)`, which raises
ahh I see you are closing an issue (xref my last comment), can you add a whatsnew note describing the change.
Do any of these (median) need docstrings? I believe that `mean` will be inherited.

May need to add these to the `Attributes` section of the TimedeltaArray docstring, and add to the list in the API docs.
this got lost but I think was ok, can you rebase and we'll look again.
rebased
thanks @jbrockmendel 

I guess this could have a whatsnew note, can you in future / other PR.
Validate plot backend when setting.
State: closed

```python
In [1]: import pandas as pd

In [2]: import sys

In [3]: import types

In [4]: module = types.ModuleType("foo")

In [5]: sys.modules['foo'] = module

In [6]: pd.set_option('plotting.backend', 'foo')
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-6-123b6513deb2> in <module>
----> 1 pd.set_option('plotting.backend', 'foo')

...

~/sandbox/pandas/pandas/plotting/_core.py in _find_backend(backend)
   1588         "top-level `.plot` method."
   1589     )
-> 1590     raise ValueError(msg.format(name=backend))
   1591
   1592

ValueError: Could not find plotting backend 'foo'. Ensure that you've installed the package providing the 'foo' entrypoint, or that the package has atop-level `.plot` method.

```

Closes https://github.com/pandas-dev/pandas/issues/28163

Labels:
name: Visualization
description: plotting

Comments:
cc @datapythonista
Merging later today if there aren't any objections.
LGTM
CLN: Use ABC classes for isinstance checks, remove unnecessary imports
State: closed

Per #27353 changed ``isinstance`` checks listed in the @WillAyd script to use ABCs where it enabled removal of imports only. 

Note there may be additional files which can be cleaned as the script does not parse multi-line statements.

- [ ] closes #xxxx
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Clean

Comments:
Nice PR! Any idea for how many more there might be?
There are a large number of additional isinstance checks which could be changed (40-50 in the files that came up in your script), but I'm not sure how many of them will allow removal of the child imports -- thus won't update those per @jbrockmendel's benchmark.

I'll keep digging for more which *do* allow import removals...do you want me to add any additional to this PR or open separate? May not get it all done at once.
I'm OK with doing it incrementally in separate PRs. Just ultimately want to make sure we have an end in sight. However you feel best to manage that works!
@jbrockmendel thoughts here? Merge if happy
Thanks @addisonlynch
CLN: have Timestamp return NotImplemented
State: closed

broken off from a branch fixing #28080.

Labels:
name: Datetime
description: Datetime data dtype

Comments:
Is there a test that this raises already?
> Is there a test that this raises already?

There are tests for all the affected cases, yes.
What is your plan for a whatsnew here? I think should mention but AFAICT you have a few more follow ups to make this complete?
Shouldn’t be any user facing change here

On Fri, Aug 30, 2019 at 11:05 AM William Ayd <notifications@github.com>
wrote:

> What is your plan for a whatsnew here? I think should mention but AFAICT
> you have a few more follow ups to make this complete?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/28157?email_source=notifications&email_token=AB5UM6FDW72XILLJILKUGRDQHFONJA5CNFSM4IPXKMQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5SLWSQ#issuecomment-526695242>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AB5UM6GDUYDEPP5PKPZKI63QHFONJANCNFSM4IPXKMQA>
> .
>

thanks @jbrockmendel
DOC: Set 0.25.2 in index.rst
State: closed

Forgot this for 0.25.1

Labels:
name: Docs

Comments:
Have you historically just done this whenever creating a new whatsnew release (or around that time)? Loosely wondering if there's a better way to automate this, though low priority I would think
Yeah, this can / should be done when the new `doc/source/whatsnew/v...` is created. Automating is a bit tricky since we don't necessarily know whether we're doing another release on that branch though.

A small script like `python scripts/start_release 0.25.3` that

1. Makes the right whatsnew
2. Populates the whatsnew with the empty sections
3. Adds to whatsnew/index.rst
4. Updates index.rst.template

would be useful, but somewhat tricky to get right.
Hmm was this supposed to trigger a back port? Didn't see one come through