-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
[CLN] resolve circular Period dependency, prepare setup.py #21854
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
Changes from all commits
aa25d2e
841edcb
1a8749f
4222187
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,5 @@ | ||
# -*- coding: utf-8 -*- | ||
# flake8: noqa | ||
|
||
from .tslibs import iNaT, NaT, Timestamp, Timedelta, OutOfBoundsDatetime | ||
|
||
# TODO | ||
# period is directly dependent on tslib and imports python | ||
# modules, so exposing Period as an alias is currently not possible | ||
# from period import Period | ||
from .tslibs import ( | ||
iNaT, NaT, Timestamp, Timedelta, OutOfBoundsDatetime, Period) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,23 +24,6 @@ def is_platform_windows(): | |
return sys.platform == 'win32' or sys.platform == 'cygwin' | ||
|
||
|
||
def is_platform_linux(): | ||
return sys.platform == 'linux2' | ||
|
||
|
||
def is_platform_mac(): | ||
return sys.platform == 'darwin' | ||
|
||
|
||
min_cython_ver = '0.28.2' | ||
try: | ||
import Cython | ||
ver = Cython.__version__ | ||
_CYTHON_INSTALLED = ver >= LooseVersion(min_cython_ver) | ||
except ImportError: | ||
_CYTHON_INSTALLED = False | ||
|
||
|
||
min_numpy_ver = '1.9.0' | ||
setuptools_kwargs = { | ||
'install_requires': [ | ||
|
@@ -53,24 +36,29 @@ def is_platform_mac(): | |
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If there are no objections, I'd like to move this dictionary up above the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure |
||
|
||
|
||
min_cython_ver = '0.28.2' | ||
try: | ||
import Cython | ||
ver = Cython.__version__ | ||
_CYTHON_INSTALLED = ver >= LooseVersion(min_cython_ver) | ||
except ImportError: | ||
_CYTHON_INSTALLED = False | ||
|
||
# The import of Extension must be after the import of Cython, otherwise | ||
# we do not get the appropriately patched class. | ||
# See https://cython.readthedocs.io/en/latest/src/reference/compilation.html | ||
from distutils.extension import Extension # noqa:E402 | ||
from distutils.command.build import build # noqa:E402 | ||
from distutils.command.build_ext import build_ext as _build_ext # noqa:E402 | ||
|
||
try: | ||
if not _CYTHON_INSTALLED: | ||
raise ImportError('No supported version of Cython installed.') | ||
try: | ||
from Cython.Distutils.old_build_ext import old_build_ext as _build_ext # noqa:F811,E501 | ||
except ImportError: | ||
# Pre 0.25 | ||
from Cython.Distutils import build_ext as _build_ext | ||
from Cython.Distutils.old_build_ext import old_build_ext as _build_ext | ||
cython = True | ||
except ImportError: | ||
from distutils.command.build_ext import build_ext as _build_ext | ||
cython = False | ||
|
||
|
||
if cython: | ||
else: | ||
try: | ||
try: | ||
from Cython import Tempita as tempita | ||
|
@@ -103,27 +91,30 @@ def is_platform_mac(): | |
|
||
|
||
class build_ext(_build_ext): | ||
def build_extensions(self): | ||
@classmethod | ||
def render_templates(cls, pxifiles): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I needed to be able to call this method separately to get |
||
for pxifile in pxifiles: | ||
# build pxifiles first, template extension must be .pxi.in | ||
assert pxifile.endswith('.pxi.in') | ||
outfile = pxifile[:-3] | ||
|
||
# if builing from c files, don't need to | ||
# generate template output | ||
if cython: | ||
for pxifile in _pxifiles: | ||
# build pxifiles first, template extension must be .pxi.in | ||
assert pxifile.endswith('.pxi.in') | ||
outfile = pxifile[:-3] | ||
|
||
if (os.path.exists(outfile) and | ||
os.stat(pxifile).st_mtime < os.stat(outfile).st_mtime): | ||
# if .pxi.in is not updated, no need to output .pxi | ||
continue | ||
if (os.path.exists(outfile) and | ||
os.stat(pxifile).st_mtime < os.stat(outfile).st_mtime): | ||
# if .pxi.in is not updated, no need to output .pxi | ||
continue | ||
|
||
with open(pxifile, "r") as f: | ||
tmpl = f.read() | ||
pyxcontent = tempita.sub(tmpl) | ||
with open(pxifile, "r") as f: | ||
tmpl = f.read() | ||
pyxcontent = tempita.sub(tmpl) | ||
|
||
with open(outfile, "w") as f: | ||
f.write(pyxcontent) | ||
with open(outfile, "w") as f: | ||
f.write(pyxcontent) | ||
|
||
def build_extensions(self): | ||
# if building from c files, don't need to | ||
# generate template output | ||
if cython: | ||
self.render_templates(_pxifiles) | ||
|
||
numpy_incl = pkg_resources.resource_filename('numpy', 'core/include') | ||
|
||
|
@@ -360,7 +351,6 @@ def run(self): | |
class CheckingBuildExt(build_ext): | ||
""" | ||
Subclass build_ext to get clearer report if Cython is necessary. | ||
|
||
""" | ||
|
||
def check_cython_extensions(self, extensions): | ||
|
@@ -379,9 +369,11 @@ def build_extensions(self): | |
|
||
|
||
class CythonCommand(build_ext): | ||
"""Custom distutils command subclassed from Cython.Distutils.build_ext | ||
""" | ||
Custom distutils command subclassed from Cython.Distutils.build_ext | ||
to compile pyx->c, and stop there. All this does is override the | ||
C-compile method build_extension() with a no-op.""" | ||
C-compile method build_extension() with a no-op. | ||
""" | ||
def build_extension(self, ext): | ||
pass | ||
|
||
|
@@ -445,7 +437,6 @@ def srcpath(name=None, suffix='.pyx', subdir='src'): | |
lib_depends.append('pandas/_libs/src/util.pxd') | ||
else: | ||
lib_depends = [] | ||
plib_depends = [] | ||
|
||
common_include = ['pandas/_libs/src/klib', 'pandas/_libs/src'] | ||
|
||
|
@@ -471,8 +462,6 @@ def pxd(name): | |
|
||
tseries_depends = np_datetime_headers + ['pandas/_libs/tslibs/np_datetime.pxd'] | ||
|
||
# some linux distros require it | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm do we need this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apparently not. It has apparently sat there unused for a while. |
||
libraries = ['m'] if not is_platform_windows() else [] | ||
|
||
ext_data = { | ||
'_libs.algos': { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm now leaning towards actually moving
_FrequencyInferer
back out of cython (it came fromtseries.frequencies
previously). The only place it is used is infrequencies.infer_freq
, which itself is only used fromDatetimelikeArrayMixin
. i.e. it could reasonably be considered out-of-scope for tslibs, which is largely scalar-centric.Moving it out would allow us to get rid of the unsightly khash dependency, which is the last non-
src
pandas dependency fortslibs
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure that would be ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great, will do this in follow up