Adding support for ppc64le architecture

[anandtrex]

This is done just by updating the cpuinfo.py file from upstream

TODO:

• Unit tests and/or doctests in docstrings
• tox -e py36 passes locally
• tox -e py27 passes locally
• Docstrings and API docs for any new/modified user-facing classes and functions
• Changes documented in docs/release.rst
• tox -e docs passes locally
• AppVeyor and Travis CI passes
• Test coverage to 100% (Coveralls passes)

[alimanfoo]

Thank you @anandtrex for this. It looks like some of the Windows builds are failing to compile blosc, I haven't seen that before. I've kicked off a rebuild, will see what happens.

[jakirkham]

Is there some CI we can use to test ppc64le?

[anandtrex]

@alimanfoo Any ideas on why the windows build fails? I have no experience with windows at all, so I'm having some trouble interpreting the error messages.

[alimanfoo]

Hi @anandtrex, I'm on leave at the moment then travelling for a bit, but will try to take a look asap.

I was actually thinking about this the other day, IIRC at some point I accepted some manual edits to cpuinfo.py to resolve a problem, but I can't remember what, and in any case that was a bad idea as we should stick to keeping in sync with the upstream cpuinfo module source. Can you confirm that your PR completely overwrites our version of cpuinfo.py with latest upstream sources (I think it should)?

@jakirkham would it be an option to pull in cpuinfo via setup_requires? I'm a bit scared of that as IIUC seup_requires does some voodoo to pull in packages during a pip install, but I don't really understand what or if people think this is a bad idea these days?

[anandtrex]

Hi @alimanfoo Yes, in my PR, the cpuinfo.py is identical to the upstream one.

[alimanfoo]

Thinking about this some more, we should be able to install cpuinfo from pypi by including it in setup_requires. This would avoid having to vendor cpuinfo ourselves. I think it would be worth giving it a try, should reduce the maintenance burden in future.

[alimanfoo]

Regarding the CI failures, it might be worth waiting until #92 is merged then rebasing and seeing if any build failures remain.

[alimanfoo]

@zarr-developers/core-devs any objections to adding cpuinfo into setup_requires rather than vendoring?

[jakirkham]

IME setup_requires can be a bit flaky. Namely pip does not really support it and some setuptools versions have broken it. That said, I have used it before and if that seems like the best solution, we can do it.

A better solution would be to follow PEP 518.

[alimanfoo]

Yeah, setup_requires is such a mess. We're using it currently for setuptools-scm, so I think I figured it kinda works, but I could be wrong. Is PEP 518 implemented yet?

[alimanfoo]

Yeah, setup_requires is such a mess. We're using it currently for setuptools-scm, so I think I figured it kinda works, but I could be wrong. Is PEP 518 implemented yet?

Answering my own question, looks like it is fully supported as of pip 10.

[jakirkham]

We're using it currently for setuptools-scm, so I think I figured it kinda works, but I could be wrong.

Good point. As we've already gone down this road, then I agree using setup_requires seems reasonable. We can save the PEP 518 update for a follow-up PR then.

[alimanfoo]

Actually I wonder if PEP518 is implemented maybe we should give it a try.

…

On Tue, 20 Nov 2018, 18:27 jakirkham ***@***.*** wrote: We're using it currently for setuptools-scm, so I think I figured it kinda works, but I could be wrong. Good point. As we've already gone down this road, then I agree using setup_requires seems reasonable. We can save the PEP 518 update for a follow-up PR then. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#82 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAq8Qn-TGYkS0y_u_OQOFs8GfZFBq99Jks5uxEl3gaJpZM4UK3oT> .

[jakirkham]

It probably is worth trying. Was just trying to avoid feature creep within this PR. 😄

[alimanfoo]

Hi all, I've made a PR (#130) which uses setup_requires for cpuinfo. I tried using pyproject.toml but there are complications and I think it's worth waiting for further work to be completed upstream (e.g., in pip and tox). My proposal is to merge #130 instead of this PR for ease of maintenance. Please let me know if any objections.

[alimanfoo]

I've backed off #130 and think for the current release would be better to pursue this PR. I've merged in latest changes from master, will see if CI failures recur.

[jakirkham]

SGTM

[alimanfoo]

OK, travis is green, but appveyor is failing on PY27. Problem is with blosc compilation, this is the error:

c:\projects\numcodecs\c-blosc\blosc\blosc-common.h(77) : fatal error C1083: Cannot open include file: 'immintrin.h': No such file or directory

Looks like this issue is similar: facebook/zstd#9

[jakirkham]

If we make sure __AVX2__ is undefined on Windows Visual Studio 2008, that should work.

ref: https://github.com/Blosc/c-blosc/search?q=immintrin.h&unscoped_q=immintrin.h

[alimanfoo]

Thanks @jakirkham. So would that be the same as, in setup.py, replacing:

        if os.name == 'nt':
            define_macros += [('__AVX2__', 1)]

...with:

        if os.name == 'nt' and not PY2:
            define_macros += [('__AVX2__', 1)]

...?

[jakirkham]

Sure, that seems reasonable.

[alimanfoo]

OK, cool, I've disabled AVX2 on PY2 windows, if CI passes I'll merge.

[alimanfoo]

Trying a better way to disable AVX2 on windows PY2...

[jakirkham]

Thanks @anandtrex and @alimanfoo 😄