Fix unable to build Pandas with xlc on z/OS
State: closed

- [x] closes #35826
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

This is an attempt at being able to build Pandas using xlc on z/OS. There's two issues being fixed here:

1) Raise the default standard that being used with xlc++ (`-qlanglvl`) only when compiling c++ code, and fix other macros that otherwise cause errors when compiling with that option. I put the change directly above where it's used, but an alternate spot that it could go is directly within the declarations of each cpp file + it's respective macros. But since this would need to be added to any additional c++ code that's added, and all extra options are the same, I left it here. 

2) Fix the compilation errors that occur - it's currently missing two functions that aren't in the std:: namespace. The first is that `signbit` is declared as a macro and not a function, so we cannot redeclare the function without changing the macro. The next, `isnan`, is declared as both a macro and a function. According to the [xlc docs](https://www.ibm.com/support/knowledgecenter/en/SSLTBW_2.4.0/com.ibm.zos.v2r4.bpxbd00/risnan.htm), to use the function you can simply undefine `isnan`, which is done here, and then it's also placed in the std namespace. To use the macro form we could take the same approach as signbit.

If we don't want to touch the std namespace, an alternative could be to swap the declarations (for z/OS only) within `pandas/_libs/window/aggregations.pyx` to not use std for those two functions.

Labels:
name: Build
description: Library building on various platforms
name: CI
description: Continuous Integration

Comments:
The test failures from the CI build seem to the same as on master.
How common is z/OS?  Is it worthwhile/feasible to test it in the CI?
We've been seeing lots of interest from users in being able to run Pandas on z/OS, and xlc (which this PR fixes) is the most common compiler found on that platform. For CI - on z/OS we're unable to use miniconda/anaconda right now, so at least in it's current form this is not feasible yet - but it's something that we're looking into getting fixed.
Hi Steve, I am with Rocket Software.  We have a z/OS Miniconda in beta now.  Should be released in a couple weeks.  We also have ported Python and pandas to z/OS.  If the pandas community is interested in formally adopting z/OS as a platform we would be happy to offer our sources to assist.  Which python are you using on z/OS?  IBM recently released their own Python port.  You can reach me directly at pfandel@rocketsoftware.com.  -Peter
@PeterFandelAtRocket see https://github.com/pandas-dev/pandas/issues/33971 for some discussion. "Officially" supporting a platform would need (binary) NumPy packages and freely available CI systems ( ideally through TravisCI or azure-pipelines since that's what we're using), and a champion to handle issues as they come in.
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.
@dsaxton I didn't follow https://github.com/pandas-dev/pandas/pull/36336 closely, but were we able to implement "only mark as stale if it's awaiting review"? The last action here was a commit, so ideally we wouldn't get a stale label.
> @dsaxton I didn't follow #36336 closely, but were we able to implement "only mark as stale if it's awaiting review"? The last action here was a commit, so ideally we wouldn't get a stale label.

There are some labels that can be added to make it ignore the PR; right now those are "Needs Review", "Blocked", and "Needs Discussion'". I'm not sure if there's a way of setting conditions based on who made the last action.
Is there anything that I could do/provide to help get a reviewer to take a look at this PR?
@pitmanst the content here looks pretty benign; the limiting factor is our inability to include this in the CI.  If we had a good strategy to make that feasible, this would move much more quickly.  Absent that, @TomAugspurger is there a plan B?
Is there a way to automatically test support for the OS? Perhaps a docker container that can be set up in CI? #30641 may be a good template to work off of
@pitmanst happy to take this if you can merge master, move the note to 1.3, and confirm that this works. note we won't have official support, but can mention this as experimental.
@pitmanst can you merge master to resolve conflicts
> @pitmanst can you merge master to resolve conflicts

Thanks @pitmanst for updating

> @pitmanst happy to take this if you can merge master, move the note to 1.3, and confirm that this works. note we won't have official support, but can mention this as experimental.

Is this now ready for review?




Thanks for taking it into consideration. The failures that we see on the checks seem to be unrelated to this change. It should be ready for review.
I've updated this again, and looks like all checks are passing now. Would someone be able to review this?
Thanks for the review @jreback. I've addressed the comments and the checks are green.
Hello @pitmanst! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:



There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2021-05-13 19:22:56 UTC
Thanks @pitmanst, great changes, look much clearer and compact now IMO. Can you run `black setup.py` to see if it solves the with the `if` line being too long? Otherwise you'll have to splt it in multiple lines manually (and then also run black). Looks good to me after that.

I guess there is not CI (external to pandas) that can test this before we merge it, right? 
Thanks for the reviews @datapythonista! It didn't look like `black pandas` fixed it, so I just manually did it. There is no CI for this currently - I've been testing the changes myself. 
For CI - is there any documentation anywhere on what's required (env, any packages, how to connect) to add a machine to the CI (i.e. a z/OS machine)? I see that it's running on Azure, but any extra docs would be helpful.
Thanks for the fix @pitmanst. I think in Python is preferred to use brackets instead os backslashes for multiline, Like:
```python
foo = (long_var
       + next_line_var)

foo = long_var + \
    next_line_var
```
And also, I think it's more readable to have a condition per line.

No big deal, but if we need to iterate further, feel free to check and see if that makes the code more readable.

For the CI, probably more than documentation you can have a look at `ci/setup_env.sh`.
Thanks for the pointers on CI - I'll take a look. As for the change - I've applied the suggested change.
thanks @pitmanst