Skip to content

BLD: list proper deps for tslib #18117

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 5, 2017
Merged

BLD: list proper deps for tslib #18117

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4281ff8
BLD: list proper deps for tslib
jreback Nov 4, 2017
2fa2de5
moar
jreback Nov 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 29 additions & 6 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,8 @@ def pxd(name):
'pandas/_libs/src/datetime/np_datetime_strings.h']
np_datetime_sources = ['pandas/_libs/src/datetime/np_datetime.c',
'pandas/_libs/src/datetime/np_datetime_strings.c']
tseries_depends = np_datetime_headers + ['pandas/_libs/src/datetime.pxd']
tseries_depends = np_datetime_headers + ['pandas/_libs/src/datetime.pxd',
'pandas/_libs/tslibs/np_datetime.pxd']

# some linux distros require it
libraries = ['m'] if not is_platform_windows() else []
Expand Down Expand Up @@ -522,6 +523,10 @@ def pxd(name):
'pandas/_libs/src/parser/io.c']},
'_libs.period': {
'pyxfile': '_libs/period',
'pxdfiles': ['_libs/src/util',
'_libs/lib',
'_libs/tslibs/timezones',
'_libs/tslibs/nattype'],
'depends': tseries_depends + ['pandas/_libs/src/period_helper.h'],
'sources': np_datetime_sources + ['pandas/_libs/src/period_helper.c']},
'_libs.properties': {
Expand All @@ -535,15 +540,24 @@ def pxd(name):
'depends': _pxi_dep['sparse']},
'_libs.tslib': {
'pyxfile': '_libs/tslib',
'pxdfiles': ['_libs/src/util'],
'pxdfiles': ['_libs/src/util',
'_libs/src/khash',
'_libs/tslibs/conversion',
'_libs/tslibs/timedeltas',
'_libs/tslibs/timezones',
'_libs/tslibs/nattype'],
'depends': tseries_depends,
Copy link
Member

Choose a reason for hiding this comment

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

Does it matter that np_datetime is included in 'depends' via tseries_depends but not in 'pxdfiles'?

Putting e.g. conversion.pxd into tslib's 'pxdfiles' entry adds it to the sources kwarg of Extension. Why is that the desired outcome? (and uh, what does that do? the distutils docs don't actually say anything useful)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the disutils docs are pretty bad.

I went with the premise that if we are cimporting then we care if the api changes. to test is easy, change something in nattype.pxd, you get recompiles in lots of things, but not everything (e.g. fields/frequencies and such). change timedeltas.pxd and you get less.

this has the desired behavior of only dependency based compilation.

I think the sources/depends are actually much more on the c-level and are separate from this. (but I don't think this is really an issue as we basically include _libs/src in the compilation each time so that the c-headers are there).

'sources': np_datetime_sources},
'_libs.tslibs.conversion': {
'pyxfile': '_libs/tslibs/conversion',
'pxdfiles': ['_libs/src/util',
'_libs/tslibs/timezones',
'_libs/tslibs/timedeltas'],
'depends': tseries_depends,
'sources': np_datetime_sources},
'_libs.tslibs.fields': {
'pyxfile': '_libs/tslibs/fields',
'pxdfiles': ['_libs/src/util'],
Copy link
Member

Choose a reason for hiding this comment

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

The functions defined in util are evidently copy/pasted in the generated C code here even without this entry being present. See previous question about "what is this intended to do?"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think these are orthogonal, e.g. the dependency based compilation and the actual c code generation.

'depends': tseries_depends,
'sources': np_datetime_sources},
'_libs.tslibs.frequencies': {
Expand All @@ -557,18 +571,27 @@ def pxd(name):
'depends': np_datetime_headers,
'sources': np_datetime_sources},
'_libs.tslibs.offsets': {
'pyxfile': '_libs/tslibs/offsets'},
'pyxfile': '_libs/tslibs/offsets',
'pxdfiles': ['_libs/src/util',
'_libs/tslibs/conversion']},
'_libs.tslibs.parsing': {
'pyxfile': '_libs/tslibs/parsing',
'pxdfiles': ['_libs/src/util']},
'pxdfiles': ['_libs/src/util',
'_libs/src/khash']},
'_libs.tslibs.strptime': {
'pyxfile': '_libs/tslibs/strptime',
'pxdfiles': ['_libs/src/util',
'_libs/tslibs/nattype'],
'depends': tseries_depends,
'sources': np_datetime_sources},
'_libs.tslibs.timedeltas': {
'pyxfile': '_libs/tslibs/timedeltas'},
'pyxfile': '_libs/tslibs/timedeltas',
'pxdfiles': ['_libs/src/util'],
'depends': np_datetime_headers,
'sources': np_datetime_sources},
'_libs.tslibs.timezones': {
'pyxfile': '_libs/tslibs/timezones'},
'pyxfile': '_libs/tslibs/timezones',
'pxdfiles': ['_libs/src/util']},
'_libs.testing': {
'pyxfile': '_libs/testing'},
'_libs.window': {
Expand Down