aggregations.pyx: use std:: for signbit() and sqrt().
State: closed

This reintroduces how this was done in pandas 1.3.5. This should fix issue #51047.

- [x] closes #51047
- [ ] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [ ] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [ ] Added [type annotations](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#type-hints) to new arguments/methods/functions.
- [ ] Added an entry in the latest `doc/source/whatsnew/vX.X.X.rst` file if fixing a bug or adding a new feature.


Labels:
name: Window
description: rolling, ewma, expanding
name: Stale

Comments:
Can we get this from numpy?

worth up streaming a fix to cython?
Thanks a ton for the research. I agree with @jbrockmendel though I think this change should be done upstream in Cython
> Thanks a ton for the research. I agree with @jbrockmendel though I think this change should be done upstream in Cython

Really?  Well, OK.  Who takes that up with the Cython maintainers?  Myself I barely know python, and much less Cython, so feel kind of lost.
Hmm on second thought this might be something we need to handle on our end. What is importing the C++ complex header? Is that done via deque?
> Hmm on second thought this might be something we need to handle on our end. What is importing the C++ complex header? Is that done via deque?

The generated `aggregations.cpp` contains

```
/* Header.proto */
#if !defined(CYTHON_CCOMPLEX)
  #if defined(__cplusplus)
    #define CYTHON_CCOMPLEX 1
  #elif defined(_Complex_I)
    #define CYTHON_CCOMPLEX 1
  #else
    #define CYTHON_CCOMPLEX 0
  #endif
#endif
#if CYTHON_CCOMPLEX
  #ifdef __cplusplus
    #include <complex>
  #else
    #include <complex.h>
  #endif
#endif
```
but that is long after
```
#include "Python.h"
```
in the generated `aggregations.cpp` file.  `Python.h` includes `pyport.h`, which in turn includes `<math.h>`.  So, since this is a C++ file, `<complex>` gets included, long after `<math.h>` is included.

`CYTHON_CCOMPLEX` is not defined elsewhere in the `agggrergations.cpp` file, and therefore, on NetBSD at least, you end up with `signbit()` and `sqrt()` only being available via the C++ `std::` name space at that point.

This is with Python 3.10 on NetBSD/macppc 10.0_BETA and cython 0.29.33.

Awesome - appreciate all the details again. Yea this is a bit tricky - I see why your solution works but (especially since we don't have CI set up for your issue) am wary of how fragile it will be for us over time.

Does importing as an alias from Cython have any effect, i.e. doing `from libc.math import signbit as c_signbit`? Or do things still get clobbered that way

@scoder may have a better solution
This pull request is stale because it has been open for thirty days with no activity. Please [update](https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#updating-your-pull-request) and respond to this comment if you're still interested in working on this.
There's a PR for adding `libcpp.cmath` declarations to Cython 3.0: https://github.com/cython/cython/pull/5262

> `from libc.math import signbit as c_signbit`

This won't change anything in the generated C/C++ code.

Since yo appear to be generating C++ code here, it seems reasonable to use the C+ math functions from their respective namespace. That's generally safer than using something prone to naming collisions from the flat global C namespace.
Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen.
I'm sorry, I ran out of energy trying to understand what would be your alternative suggestions.  To me it seems "more correct" and less ambigious and less dependent on the particular layout of the C header files to use the `std::` versions of these functions when doing so from C++, as is the case here.
Sorry lost in the shuffle but from the conversation I think this is ok. If you want to meet in main and get the CI green I would approve
Err wait...from the PR linked above it looks like Cython has libcpp.math - can you import from there now?