BLD: list proper deps for tslib #18117
Conversation
|
@jbrockmendel I believe this accomplishes the goal of explicitly laying out depedency tree. Note that we don't really need to specify transitive ones (e.g. util), so that's a bit sloppy here. but I think its ok. These deps should be reflective of the api's that modules exports (via the .pxd) to others. |
| '_libs/tslibs/conversion', | ||
| '_libs/tslibs/timedeltas', | ||
| '_libs/tslibs/timezones', | ||
| '_libs/tslibs/nattype'], | ||
| 'depends': tseries_depends, |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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).
setup.py
Outdated
| 'depends': tseries_depends, | ||
| 'sources': np_datetime_sources}, | ||
| '_libs.tslibs.conversion': { | ||
| 'pyxfile': '_libs/tslibs/conversion', | ||
| 'pxdfiles': ['_libs/src/util', | ||
| '_libs/tslibs/timezones', | ||
| '_libs/tslibs/np_datetime', |
There was a problem hiding this comment.
np_datetime is doubled up here and in tseries_depends.
| 'depends': tseries_depends, | ||
| 'sources': np_datetime_sources}, | ||
| '_libs.tslibs.fields': { | ||
| 'pyxfile': '_libs/tslibs/fields', | ||
| 'pxdfiles': ['_libs/src/util'], |
There was a problem hiding this comment.
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?"
There was a problem hiding this comment.
I think these are orthogonal, e.g. the dependency based compilation and the actual c code generation.
Codecov Report
@@ Coverage Diff @@
## master #18117 +/- ##
==========================================
- Coverage 91.25% 91.23% -0.02%
==========================================
Files 163 163
Lines 50123 50124 +1
==========================================
- Hits 45741 45733 -8
- Misses 4382 4391 +9
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #18117 +/- ##
==========================================
- Coverage 91.25% 91.23% -0.02%
==========================================
Files 163 163
Lines 50123 50124 +1
==========================================
- Hits 45741 45733 -8
- Misses 4382 4391 +9
Continue to review full report at Codecov.
|
closes #18089