Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0.6.4 install fails on Mac OSX, Windows CI machines #210

Closed
sofroniewn opened this issue Nov 8, 2019 · 23 comments
Closed

0.6.4 install fails on Mac OSX, Windows CI machines #210

sofroniewn opened this issue Nov 8, 2019 · 23 comments

Comments

@sofroniewn
Copy link

Minimal, reproducible code sample, a copy-pastable example if possible

We (napari/napari#665) are running into errors installing dask / zarr on our mac osx and windows CI due to the recent 0.6.4 release. On our linux CI it installs fine. For mac and windows we end with the following, but please see the links above for the full stack traces:

  ERROR: Failed building wheel for numcodecs

Problem description

Expected outcome is successful install

Version and installation information

Please provide the following:

  • Value of numcodecs.__version__ 0.6.4
  • Version of Python interpreter: 3.6 and 3.7
  • Operating system: Windows and Mac
  • How NumCodecs was installed: pip in our CI
@sofroniewn sofroniewn changed the title 0.6.4 install on Mac OSX, Windows 0.6.4 install fails on Mac OSX, Windows Nov 8, 2019
@sofroniewn sofroniewn changed the title 0.6.4 install fails on Mac OSX, Windows 0.6.4 install fails on Mac OSX, Windows CI machines Nov 8, 2019
@jakirkham
Copy link
Member

I've responded in the issue.

FWIW we also test builds on macOS (as of this release in fact) and Windows. So I think once the CI issue is sorted things will start working for you.

@jni
Copy link

jni commented Nov 8, 2019

@jakirkham any chance of numcodecs uploading wheels to PyPI? As mentioned in the issue, users are having issue installing it, too.

@jakirkham
Copy link
Member

Please see issue ( #70 ).

@jni
Copy link

jni commented Nov 9, 2019

Fun! 😬

@jakirkham
Copy link
Member

Maybe this ( #70 (comment) ) helps?

@jakirkham
Copy link
Member

Hmm...thinking about this a bit more it doesn't appear Blosc has changed the version of Snappy they are using. So that's not likely the issue

We use to skip over compilation failures and then just not provide the C extensions. However we changed that recently as users were unhappy not having a clearer error. So it is likely this was never working on macOS and Windows and we are now recently seeing this.

So I think there is probably something off about how we are building the Snappy portion of the extension in particular. I'll see if I can reproduce it and report back.

@jakirkham jakirkham mentioned this issue Nov 9, 2019
8 tasks
@jakirkham
Copy link
Member

Was unable to reproduce the issue unfortunately.

My best guess is PR ( #211 ) fixes the macOS issue. Though really need feedback to be sure.

The Windows error seems to be complaining about Visual Studio not being installed. So that would need to be installed by the user.

@jni
Copy link

jni commented Nov 9, 2019

@jakirkham but 0.6.3 wasn't causing any issues, and users were happily using napari. Is there a way to not depend on this compilation in future releases? e.g. a hard warning, or a numcodecs-lite module that zarr could depend on?

@jakirkham
Copy link
Member

jakirkham commented Nov 9, 2019

Yeah that's fair.

We decided to make the errors hard in PR ( #197 ) after some discussion.

We could revert that PR. Perhaps users wanting to fail the build could set an environment variable to get that behavior. WDYT @llllllllll would this work for your use case?

@jni
Copy link

jni commented Nov 9, 2019

@jakirkham this was interesting:

This change just removes the fallback, if users would like to install without
extensions, they must pass the (existing) DISABLE_NUMCODECS_CEXT=1 envvar.

Is this something that can be done in the napari source distribution, so that if the user is pip installing napari, this will work? That would be a nice solution. I understand totally concerns that silent failure is not great.

However, what we really want is to mark compilation as optional, as before: if a user has a compiler and it works, great, if not, well, from napari's perspective, we don't really care (until someone complains 😛). But we don't necessarily want to go with a saddled numcodecs if it's not needed.

I kinda think numcodecs-lite would work well until wheels are available, because then there is a clear error message to display to users who need the advanced codecs: please install numcodecs, and those users can go through the trouble of installing compilers etc, but most users can go on blissfully with their lives...

@jakirkham
Copy link
Member

@jakirkham this was interesting:

This change just removes the fallback, if users would like to install without
extensions, they must pass the (existing) DISABLE_NUMCODECS_CEXT=1 envvar.

Is this something that can be done in the napari source distribution, so that if the user is pip installing napari, this will work? That would be a nice solution. I understand totally concerns that silent failure is not great.

Yep, this is a thing you could do.

However, what we really want is to mark compilation as optional, as before: if a user has a compiler and it works, great, if not, well, from napari's perspective, we don't really care (until someone complains 😛). But we don't necessarily want to go with a saddled numcodecs if it's not needed.

I kinda think numcodecs-lite would work well until wheels are available, because then there is a clear error message to display to users who need the advanced codecs: please install numcodecs, and those users can go through the trouble of installing compilers etc, but most users can go on blissfully with their lives...

Yep, I get that. For clarity what I'm proposing is...

  1. Revert the patch
  2. Have users that want a failure opt-in (as opposed to others opt-out)
  3. Users get a warning about extensions not compiling and life continues

@jakirkham
Copy link
Member

That said, I do want to hear feedback from other users. Maybe there are other things we are missing. 🙂

@llllllllll
Copy link
Contributor

Perhaps users wanting to fail the build could set an environment variable to get that behavior.

The issue is that people don't know that they would need to do this because soft compile errors are non-standard. I don't think pip lets you warn from a setup.py, so there is no way to let the user know something has failed until your code crashes with an importerror. If you get an error, then you know it doesn't work and can pass the variable to disable the extensions and try again. Maybe we need to enhance the error message to tell people about this flag. We could also add something like NUMCODECS_SILENT_COMPILE_ERRORS=1 to get back to the old behavior up front.

@jni
Copy link

jni commented Nov 9, 2019

Yep, this is a thing you could do.

... how? I thought it would be quick to find information on setting environment variables in e.g. pyproject.toml but I don't see how. I suspect that messing with os.environ within a setup.py won't affect the environment when dependencies are being installed, though I don't actually know this...

@jakirkham
Copy link
Member

The issue is that people don't know that they would need to do this because soft compile errors are non-standard.

For sure. I don't love it.

That said, we are now getting an uptick in novice users and they are running into compilation issues, which is not great either. Please see issue ( napari/napari#665 ) for context.

I don't think pip lets you warn from a setup.py, so there is no way to let the user know something has failed until your code crashes with an importerror.

I think if we print things and they use verbose a message gets out, but yeah you are right pip doesn't make this easy.

If you get an error, then you know it doesn't work and can pass the variable to disable the extensions and try again. Maybe we need to enhance the error message to tell people about this flag. We could also add something like NUMCODECS_SILENT_COMPILE_ERRORS=1 to get back to the old behavior up front.

Yeah it's worth at least improving the existing message. Though is there something more we can do here?

@jakirkham
Copy link
Member

Yep, this is a thing you could do.

... how? I thought it would be quick to find information on setting environment variables in e.g. pyproject.toml but I don't see how. I suspect that messing with os.environ within a setup.py won't affect the environment when dependencies are being installed, though I don't actually know this...

I see, you mean in the build scripts of your project? No I'm not aware of anything offhand. Something may exist, but it probably requires more digging.

That said, users could set the environment variable externally before running pip, which should workaround the problem.

@jni
Copy link

jni commented Nov 9, 2019

That said, users could set the environment variable externally before running pip, which should workaround the problem.

Ah sure! But no, my priority is that users can just type pip install napari and things will Just Work. I am 100% sympathetic to the idea that compilation errors should not just pass silently by default, but I hope we can get to a solution that does not require us to pin numcodecs forever! I still think a two-tiered release, numcodecs and numcodecs[all], might meet everyone's requirements? Though yes, it is definitely more work for maintainers...

Wheels would make this whole conversation moot, but I'll leave that discussion to #70.

@llllllllll
Copy link
Contributor

I think pip install numcodecs[blosc] or [all] would also cover my needs. if there was some note in the docs for the blosc codec or or some of the other compiled codecs, then when a new user gets the import error they will quickly find that note and move on. I assume napari doesn't use blosc or the other compiled codecs, so not installing them isn't an issue, right?

@jakirkham
Copy link
Member

If someone is willing to do that work and maintain it, then it would be ok with me too.

@jeromekelleher
Copy link
Member

Just weighing in here to support @llllllllll's perspective: I maintain some downstream packages that use the blosc codecs, and get hit by novice users installing from pip and not noticing anything is wrong until import fails. It seems like adding numcodecs[blosc] would be quite a lot of extra work though, since we'd probably want some error messaging around trying to import blosc when the extras aren't present.

Probably the only real fix for this is to have windows and OSX wheels? Wheels for linux would be nice, but failing when there's no compiler on linux is totally fine IMO.

@jakirkham
Copy link
Member

Yeah @jeromekelleher that's what we are discussing in issue ( #70 ). That or using wheels from some of our dependencies (like Blosc) to simplify the installation process here.

@jakirkham
Copy link
Member

As of 0.7.1, we now build and publish wheels from CI to PyPI for macOS, Linux, and Windows. Would encourage people to give those a try and report back.

Going to go ahead and close this (assuming wheels have fixed the issue), though please feel free to reopen (or ping us to do so) if not. Thanks!

@joshmoore
Copy link
Member

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants