Update imports, use nogil version of sqrt
State: closed


Labels:
name: Clean

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

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

```diff
@@            Coverage Diff             @@
##           master   #18557      +/-   ##
==========================================
- Coverage   91.35%   91.33%   -0.02%     
==========================================
  Files         164      164              
  Lines       49802    49800       -2     
==========================================
- Hits        45496    45485      -11     
- Misses       4306     4315       +9
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.13% <88.88%> (-0.01%)` | :arrow_down: |
| #single | `40.81% <55.55%> (-0.07%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/18557?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/tools/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/18557?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvdG9vbHMvZGF0ZXRpbWVzLnB5) | `84.43% <ø> (-0.06%)` | :arrow_down: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/18557?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `95.71% <100%> (-0.01%)` | :arrow_down: |
| [pandas/tseries/offsets.py](https://codecov.io/gh/pandas-dev/pandas/pull/18557?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvb2Zmc2V0cy5weQ==) | `96.9% <50%> (ø)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/18557?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/18557?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.81% <0%> (-0.1%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18557?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/18557?src=pr&el=footer). Last update [32f562d...703a54b](https://codecov.io/gh/pandas-dev/pandas/pull/18557?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/18557?src=pr&el=h1) Report
> Merging [#18557](https://codecov.io/gh/pandas-dev/pandas/pull/18557?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/bd9a3e06621e0b2bff3a7f4a6bf68c886b0db4c3?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `88.88%`.

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

```diff
@@            Coverage Diff             @@
##           master   #18557      +/-   ##
==========================================
+ Coverage   91.44%   91.45%   +<.01%     
==========================================
  Files         157      157              
  Lines       51449    51447       -2     
==========================================
+ Hits        47048    47049       +1     
+ Misses       4401     4398       -3
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.32% <88.88%> (+0.02%)` | :arrow_up: |
| #single | `40.6% <55.55%> (-0.11%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/18557?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/tools/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/18557/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvdG9vbHMvZGF0ZXRpbWVzLnB5) | `84.43% <ø> (-0.06%)` | :arrow_down: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/18557/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `95.68% <100%> (-0.01%)` | :arrow_down: |
| [pandas/tseries/offsets.py](https://codecov.io/gh/pandas-dev/pandas/pull/18557/diff?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvb2Zmc2V0cy5weQ==) | `96.9% <50%> (ø)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/18557/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/18557/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.81% <0%> (-0.1%)` | :arrow_down: |
| [pandas/plotting/\_converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/18557/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb252ZXJ0ZXIucHk=) | `65.25% <0%> (+1.81%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18557?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/18557?src=pr&el=footer). Last update [bd9a3e0...e743c1e](https://codecov.io/gh/pandas-dev/pandas/pull/18557?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

would it not be better to expose this as an definition in a single .pxd, maybe call it cmath.pxd (along with other ones), then simply import centrally from there (or could also just add to util.pxd).
> would it not be better to expose this as an definition in a single .pxd, maybe call it cmath.pxd (along with other ones), then simply import centrally from there (or could also just add to util.pxd).

I see the upside to this but also like transparent/direct imports.  Not sure there is enough content to justify a cmath.pxd; what else would go in there?

I'll open an issue with cython to see if they can fix the declaration on their end too and make this (eventually) a non-issue.

For purposes of expediency, would it be easier if I revert the sqrt bits now and get the easy other fixups closed down?
i would put it in util.pxd

the issue is it’s very likely that future changes will accidentally just use the wrong import and just do it with d
conda HTTPError 502 on Appveyor.  Will wait until late-evening so re-push as the build queue is usually short then.
looks fine. prefer that you update all of these in this pr.
Travis error in 3.6 TestDataFramePlots.test_parallel_coordinates_with_sorted_labels
> prefer that you update all of these in this pr.

I opened an issue on the cython tracker and based on [the response](https://github.com/cython/cython/issues/2029#issuecomment-348006128) I think we should stick a pin in that until we have a better idea what's going on.
are these in cython 0.26 or greater?
> are these in cython 0.26 or greater?

All my local builds should be on 0.27.1
and what is the perf affect of this
specifically related to the revert of the nogil sqrt changes
> specifically related to the revert of the nogil sqrt changes

The "revert" is reverting from an earlier version of the PR, i.e. keeps the status quo in master unchanged.  This should be nothing but import cleanup.
thanks!