BLD: fix inline warnings
State: closed

Previous PR: https://github.com/pandas-dev/pandas/pull/17277

Labels:
name: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
perf tests, just to confirm this doesn't change anything. don't have to run them all, maybe a subset esp indexing.
Test error looks unrelated to the PR.  Is this correct?
```
asv continuous -f 1.1 -E virtualenv master HEAD -b indexing
[...]
        before           after         ratio
     [fa557f73]       [a00dc84b]
+      14.9±0.4ms         190±20ms    12.73  indexing.Int64Indexing.time_getitem_lists
+      34.5±0.4μs        60.3±20μs     1.75  indexing.IndexingMethods.time_get_loc_float
+      10.7±0.7ms         17.6±1ms     1.64  indexing.IndexingMethods.time_take_dtindex
+        880±30μs      1.42±0.02ms     1.62  indexing.Int64Indexing.time_getitem_list_like
+     2.99±0.05ms      4.26±0.05ms     1.43  indexing.DataFrameIndexing.time_loc_dups
+      14.1±0.4μs         19.6±1μs     1.39  indexing.DatetimeIndexing.time_getitem_scalar
+      7.33±0.2ms         10.2±2ms     1.39  indexing.IndexingMethods.time_take_intindex
+      46.9±0.8μs         63.6±5μs     1.35  indexing.Int64Indexing.time_getitem_scalar
+         135±2μs          172±4μs     1.27  indexing.Int64Indexing.time_getitem_slice
+       359±0.5ms          442±3ms     1.23  indexing.MultiIndexing.time_multiindex_get_indexer
+         160±3μs        189±0.7μs     1.18  indexing.Int64Indexing.time_iloc_array
+     1.10±0.01ms      1.27±0.01ms     1.16  indexing.DataFrameIndexing.time_boolean_rows_object
+         380±5ms          439±8ms     1.16  frame_methods.Iteration.time_iteritems_indexing
+         641±9μs         733±10μs     1.14  indexing.DataFrameIndexing.time_boolean_rows
+      93.1±0.6μs          105±1μs     1.12  indexing.Int64Indexing.time_iloc_list_like
+      30.1±0.5μs       33.4±0.7μs     1.11  indexing.Int64Indexing.time_iloc_scalar
-      38.8±0.5μs       29.7±0.4μs     0.77  indexing.DataFrameIndexing.time_getitem_scalar
-       202±0.4μs          153±5μs     0.76  indexing.Int64Indexing.time_loc_scalar
-      2.94±0.3ms      2.19±0.09ms     0.75  indexing.Int64Indexing.time_getitem_array
-      3.51±0.1ms      2.51±0.05ms     0.71  indexing.Int64Indexing.time_ix_array
-      54.2±0.8μs         37.9±1μs     0.70  period.period_standard_indexing.time_get_loc
-     1.11±0.02ms          775±3μs     0.70  indexing.MultiIndexing.time_series_xs_mi_ix
-     24.6±0.01μs         16.8±2μs     0.68  indexing.MultiIndexing.time_multiindex_med_get_loc
-         727±4ms          487±3ms     0.67  reindex.Reindexing.time_reindex_multiindex
-        788±40ms          519±7ms     0.66  indexing.StringIndexing.time_get_value
-     1.48±0.04ms          967±6μs     0.65  indexing.Int64Indexing.time_ix_list_like
-     3.94±0.07ms       2.57±0.3ms     0.65  indexing.PanelIndexing.time_subset
-        904±30ms          588±4ms     0.65  indexing.StringIndexing.time_getitem_label_slice
-     2.02±0.06ms      1.31±0.02ms     0.65  indexing.Int64Indexing.time_loc_list_like
-       962±100μs         621±30μs     0.65  period.period_standard_indexing.time_intersection
-           1.21s          774±6ms     0.64  indexing.MultiIndexing.time_multiindex_large_get_loc_warm
-     1.97±0.08μs      1.24±0.02μs     0.63  period.period_standard_indexing.time_shape
-        242±20μs         151±10μs     0.62  indexing.IntervalIndexing.time_getitem_scalar
-        300±10μs          181±5μs     0.60  indexing.StringIndexing.time_getitem_pos_slice
-      21.1±0.3ms       12.5±0.4ms     0.60  indexing.MultiIndexing.time_multiindex_slicers
-      7.27±0.3ms       4.19±0.3ms     0.58  indexing.Int64Indexing.time_loc_array
-       360±200μs          201±4μs     0.56  period.period_standard_indexing.time_series_loc
-        52.1±3μs       28.7±0.9μs     0.55  period.period_standard_indexing.time_shallow_copy
-        163±10μs         88.7±2μs     0.54  indexing.Int64Indexing.time_ix_scalar
-     2.21±0.01ms      1.17±0.04ms     0.53  indexing.MultiIndexing.time_remove_unused_levels
-        30.2±2μs       14.6±0.1μs     0.48  indexing.MultiIndexing.time_multiindex_string_get_loc
-        548±10μs         256±10μs     0.47  indexing.IntervalIndexing.time_getitem_list
-        545±10μs         250±10μs     0.46  indexing.IntervalIndexing.time_loc_scalar
-        460±40μs         190±20μs     0.41  indexing.Int64Indexing.time_ix_slice
-        330±10μs          135±1μs     0.41  indexing.Int64Indexing.time_loc_slice
-          36.4ms       14.6±0.4ms     0.40  indexing.MultiIndexing.time_multiindex_med_get_loc_warm
-          43.1ms       16.2±0.7ms     0.38  indexing.MultiIndexing.time_multiindex_small_get_loc_warm
-      4.16±0.4ms      1.19±0.01ms     0.29  reindex.Reindexing.time_reindex_columns
-     2.89±0.02ms         799±20μs     0.28  reindex.Reindexing.time_reindex_dates
-         201±1ms         991±20ns     0.00  indexing.MultiIndexing.time_is_monotonic

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
```
Test failure looks unrelated.  Pls confirm.
so the asv is too flaky, pls run again. Its very important that this doesn't change much for this PR>
I'm not comfortable with the heuristic "let's cherry-pick the best ASV run".  My understanding is that removing the "inline" annotations should do nothing but remove the warnings at build time, but my understanding is not especially reliable in this area.

If we aren't collectively confident in our ability to reason about this code, then a) we shouldn't be touching it, and b) we have a bigger problem than annoying warnings in the build log.
In the previous PR, you referenced a cython discussion (https://github.com/cython/cython/issues/1706#issuecomment-302347613) which gives a nice summary.

Assuming that is correct, I think we can 'reason' about this PR (athough a less flaky asv to confirm our reasoning would still be good)

Eg taking `kh_destroy_pymap`: function declared in khash.pxd: used in hashtable.pyx (this is a case of: "Your function is called from a different module" -> this can never be inline, so removing 'inline' should not have any effect) 
I think the same holds true for all other cases this PR changes (but didn't check in detail)

(note I am not very familiar with this, I just tried to understand the comment in https://github.com/cython/cython/issues/1706#issuecomment-302347613)


# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=h1) Report
> Merging [#17528](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/b3087efed7321bcfbfa86143bb81e93bc4552eda?src=pr&el=desc) will **increase** coverage by `0.44%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/17528/graphs/tree.svg?height=150&token=eZ4WkYLtcO&width=650&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #17528      +/-   ##
==========================================
+ Coverage   91.19%   91.64%   +0.44%     
==========================================
  Files         163      163              
  Lines       49627    52400    +2773     
==========================================
+ Hits        45259    48020    +2761     
- Misses       4368     4380      +12
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.54% <ø> (+0.57%)` | :arrow_up: |
| #single | `40.5% <ø> (+0.24%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.77% <0%> (-0.1%)` | :arrow_down: |
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `96.28% <0%> (ø)` | :arrow_up: |
| [pandas/core/api.py](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXBpLnB5) | `100% <0%> (ø)` | :arrow_up: |
| [pandas/core/indexes/multi.py](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9tdWx0aS5weQ==) | `96.9% <0%> (ø)` | :arrow_up: |
| [pandas/core/sparse/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3BhcnNlL2ZyYW1lLnB5) | `94.7% <0%> (+0.01%)` | :arrow_up: |
| [pandas/core/indexes/category.py](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9jYXRlZ29yeS5weQ==) | `98.74% <0%> (+0.19%)` | :arrow_up: |
| [pandas/core/indexes/period.py](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9wZXJpb2QucHk=) | `93.06% <0%> (+0.28%)` | :arrow_up: |
| [pandas/core/resample.py](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzYW1wbGUucHk=) | `96.64% <0%> (+0.46%)` | :arrow_up: |
| [pandas/core/categorical.py](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvY2F0ZWdvcmljYWwucHk=) | `96.32% <0%> (+0.73%)` | :arrow_up: |
| ... and [9 more](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=footer). Last update [b3087ef...df725d3](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=h1) Report
> Merging [#17528](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/f797c1dc8d838eb9df5ede3be681949dab852148?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/17528/graphs/tree.svg?token=eZ4WkYLtcO&width=650&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #17528      +/-   ##
==========================================
- Coverage   91.22%    91.2%   -0.02%     
==========================================
  Files         163      163              
  Lines       49655    49655              
==========================================
- Hits        45297    45288       -9     
- Misses       4358     4367       +9
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.99% <ø> (ø)` | :arrow_up: |
| #single | `40.17% <ø> (-0.07%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.77% <0%> (-0.1%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=footer). Last update [f797c1d...0f565fc](https://codecov.io/gh/pandas-dev/pandas/pull/17528?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

> I'm not comfortable with the heuristic "let's cherry-pick the best ASV run". My understanding is that r

asv runs are generally quite stable on the same machine commit-to-commit. I find this a pretty good tool to find performance regressions. A lot of the cython code is designed to address performance issues, so using a tool for that is generally a good thing. 

So your comments here are a bit odd. 
in any event pls rebase
couple more 
```
warning: pandas/_libs/src/inference.pyx:1018:23: Declarations should not be declared inline.
warning: pandas/_libs/src/khash.pxd:16:30: Declarations should not be declared inline.
warning: pandas/_libs/src/skiplist.pxd:17:21: Declarations should not be declared inline.
warning: pandas/_libs/src/skiplist.pxd:18:32: Declarations should not be declared inline.
warning: pandas/_libs/src/skiplist.pxd:19:30: Declarations should not be declared inline.
warning: pandas/_libs/src/skiplist.pxd:20:30: Declarations should not be declared inline.
warning: pandas/_libs/src/skiplist.pxd:21:30: Declarations should not be declared inline.
warning: pandas/_libs/parsers.pyx:258:26: Declarations should not be declared inline.
```

as an aside, if you are looking for something to do..... These warnings have been on the build forever. I think you can just define it to remove, maybe need to change some amount of code (#8329).
```
/Users/jreback/miniconda3/envs/pandas/lib/python3.6/site-packages/numpy/core/include/numpy/npy_1_7_deprecated_api.h:15:2: warning: "Using deprecated NumPy API, disable it by "
      "#defining NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION" [-W#warnings]
#warning "Using deprecated NumPy API, disable it by " \
 ^
```
> couple more

Excellent.  Will push shortly.

> if you are looking for something to do

Long todo list as it is... but that warning has bugged me, so I'll probably take a swing at this anyway.

> pandas/_libs/src/skiplist.pxd

`_libs.window` "include"s skiplist.pyx and then separately cimports from skiplist.pxd (which is itself `cdef extern`ing from skiplist.h.  I think the contents of skiplist.pxd can be cut/paste into skiplist.pyx and the pxd removed.  Could be a step towards simplifying the dependency specs.  If there's consensus, I'll make a separate PR.

Sidenote: window.pyx has
```
cdef extern from "../src/headers/math.h":
    double sqrt(double x) nogil
    int signbit(double) nogil
```
That path looks wrong to me.  Is the ".." _not_ one level too many?

> warning: pandas/_libs/src/inference.pyx:1018:23: Declarations should not be declared inline

That line is

```
cdef extern from "parse_helper.h":
    inline int floatify(object, double *result, int *maybe_int) except -1
```

Which is fixed in a commit about to be pushed.  But it makes me think that the version that [util.pxd](https://github.com/pandas-dev/pandas/blob/master/pandas/_libs/src/util.pxd#L19) externs (what's the appropriate verb here?) from numpy_helper probably a) is unneeded and b) doesn't exist. 
After some googling and trial+error on the NPY_NO_DEPRECATED_API topic, I have found a new way to cause compile-time errors.

This is all FYI, not relevant to this PR.  Readers of The Future, feel free to skip.

In setup.py, passing `define_macros=[("NPY_NO_DEPRECATED_API", "NPY_1_7_API_VERSION")]` to the `Extension` constructor leads to lots of compile-time errors of the form
```
pandas/_libs/lib.c:21788:73: error: no member named 'data' in 'struct tagPyArrayObject'
    (__pyx_v_vecs[__pyx_v_i]) = ((__pyx_t_5numpy_int64_t *)__pyx_v_arr->data);
                                                           ~~~~~~~~~~~  ^
```
Based on this, I'm questioning the wording of the warning message.  `warning: "Using deprecated NumPy API, disable it by #defining NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION" [-W#warnings]`.  Is it possible that this means "disable the deprecated api" as opposed to "suppress this warning"?  Is the `[-W#warnings]` a suggestion for suppressing the warning?

thanks, this is good