Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLN: assorted (non-pytables) cleanups #29985

Merged
merged 14 commits into from
Dec 12, 2019
Merged

Conversation

jbrockmendel
Copy link
Member

includes a couple of whatsnew notes that fell through the cracks

pi = pd.date_range(start=pd.Timestamp("2000-01-01"), periods=100, freq=freqstr)
return pi
dti = pd.date_range(start=pd.Timestamp("2000-01-01"), periods=100, freq=freqstr)
return dti
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just return pd.date_range(...)

Copy link
Member

@gfyoung gfyoung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the changes look OK, but operationally speaking, I think it would have preferable to separate out some of these changes into separate PR's to facilitate review.

e.g. the whatsnew changes don't need to be held up by the eval.py changes.

@gfyoung gfyoung added Algos Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff Clean Testing pandas testing functions or related to the test suite labels Dec 3, 2019
@jbrockmendel
Copy link
Member Author

@simonjayhawkins mypy is complaining about instantiating a class where we use abc.ABCMeta, thoughts?

@jbrockmendel
Copy link
Member Author

@gfyoung broke pieces off into #30006, #30007, will revert those from this PR

@simonjayhawkins
Copy link
Member

@simonjayhawkins mypy is complaining about instantiating a class where we use abc.ABCMeta, thoughts?

try this..

diff --git a/pandas/core/computation/engines.py b/pandas/core/computation/engines.py
index a4eaa897c..9b813bb4c 100644
--- a/pandas/core/computation/engines.py
+++ b/pandas/core/computation/engines.py
@@ -3,6 +3,7 @@ Engine classes for :func:`~pandas.eval`
 """
 
 import abc
+from typing import Dict, Type
 
 from pandas.core.computation.align import align_terms, reconstruct_object
 from pandas.core.computation.ops import _mathops, _reductions
@@ -139,4 +140,7 @@ class PythonEngine(AbstractEngine):
         pass
 
 
-_engines = {"numexpr": NumExprEngine, "python": PythonEngine}
+_engines: Dict[str, Type[AbstractEngine]] = {
+    "numexpr": NumExprEngine,
+    "python": PythonEngine,
+}

@jreback
Copy link
Contributor

jreback commented Dec 6, 2019

can you rebase

@jreback jreback added this to the 1.0 milestone Dec 12, 2019
@jreback jreback merged commit 99ae56a into pandas-dev:master Dec 12, 2019
@jreback
Copy link
Contributor

jreback commented Dec 12, 2019

thanks @jbrockmendel if you can address @gfyoung comment in a followup

@jbrockmendel jbrockmendel deleted the cln-misc branch December 12, 2019 15:51
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Algos Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff Clean Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants