Skip to content

Nibabel 2.2.0 cannot be frozen #571

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

Closed
pauldmccarthy opened this issue Oct 27, 2017 · 16 comments · Fixed by #572
Closed

Nibabel 2.2.0 cannot be frozen #571

pauldmccarthy opened this issue Oct 27, 2017 · 16 comments · Fixed by #572

Comments

@pauldmccarthy
Copy link
Contributor

Hey guys,

Problem description

I've just discovered that nibabel 2.2.0 is incompatible with both pyinstaller and py2app, projects for creating standalone (a.k.a. "frozen") python applications.

Here is a stack trace from a frozen version of FSLeyes using nibabel 2.2.0:

[fsluser@localhost dist]$ ./FSLeyes/fsleyes -V
Traceback (most recent call last):
  File "site-packages/wx/core.py", line 3163, in <lambda>
  File "fsleyes/main.py", line 203, in init
  File "site-packages/nibabel/__init__.py", line 38, in <module>
  File "site-packages/nibabel/analyze.py", line 87, in <module>
  File "site-packages/nibabel/volumeutils.py", line 22, in <module>
  File "site-packages/nibabel/casting.py", line 11, in <module>
  File "site-packages/nibabel/testing/__init__.py", line 35, in <module>
  File "site-packages/nibabel/testing/np_features.py", line 19, in <module>
  File "site-packages/nibabel/testing/np_features.py", line 12, in _memmap_after_ufunc
FileNotFoundError: [Errno 2] No such file or directory: '/home/fsluser/dist/FSLeyes/nibabel/testing/np_features.pyc'

Cause

This occurs because nibabel.testing.np_features is now imported when the nibabel package is imported*. The nibabel.testing.np_features._memmap_after_ufunc function is called on import, and attempts to open the file nibabel/testing/np_features.py. The catch is that this source file doesn't exist when the code is executed from a frozen application.

* The import in the stack trace above (nibabel/casting.py) is, ironically, an unused import. But nibabel.testing is explicitly imported innibabel/__init__.py a little later on, so this error would still occur if the unused imports were removed.

Solutions

First of all, I'm not sure if this should be fixed within nibabel, or rather if it should be the responsibility of pyinstaller/py2app to explicitly handle nibabel (they have hooks for various other projects, e.g. matplotlib, numpy, etc).

Having said that, it would be easy to fix within nibabel - here are a couple of ideas:

  • Open a different file, one which is known to exist
  • Test to see if the file exists before opening it. If it doesn't, fall back to a more primitive test (e.g. checking the numpy version number)

What do you think? I'm happy to start a PR if you think that this should be handled in nibabel.

@effigies
Copy link
Member

Is this something that can be fixed by updating the MANIFEST? I don't really know the rules for that.

@pauldmccarthy
Copy link
Contributor Author

Not as far as I know - MANIFEST.in is used by distutils/setuptools to determine what files to include in a pypi source distribution. py2app and pyinstaller use runtime inspection to determine what files to include.

I'm actually related trouble with some other packages, due to some newly added dependencies in my project. I've found that with py2app it is very easy to add a special rule to include the source for a package in py2app. So far it is a complete pain with pyinstaller though - I am currently resotring to manually byte-compiling and copying .py/.pyc files into the build directory (for another project, not fornibabel)!

I suppose it comes down to where you think the responsibility lies - one of:

  • nibabel - change the code so it doesn't try to load source files
  • pyinstaller / py2app - add special hooks for all projects which behave in ways that they don't expect
  • The user (me in this case) - explicitly include source/other resources in my build

@satra
Copy link
Member

satra commented Oct 28, 2017

@pauldmccarthy - given that standard setup.py install will install both the py file and the pyc files into a python environment, perhaps this is indeed something that should be tackled at the pyinstaller/py2app level and not in nibabel.

@effigies and @matthew-brett - this function is always executed when nibabel is imported, although it is in the testing folder. is that necessary or can that be avoided?

VIRAL_MEMMAP = _memmap_after_ufunc()

matthew-brett added a commit to matthew-brett/nibabel that referenced this issue Oct 28, 2017
Avoid doing the ufunc / memmap test until we need it.

Might fix nipygh-571.
@matthew-brett
Copy link
Member

How about #572 ?

@satra
Copy link
Member

satra commented Oct 28, 2017

@pauldmccarthy - could you please check if @matthew-brett's PR branch fixes your py2app issue?

@pauldmccarthy
Copy link
Contributor Author

Hey guys,

That looks like it would do - thanks for the fast work! I can't test now unfortunately - it will have to wait until monday.

@satra It would be nice for this sort of thing to be handled by pyinstaller/py2app, but there seems to be some resistance to addressing it (e.g). But for this specific issue in nibabel, my thinking was along the same lines as your own - it's a testing function, so I don't see any real need for it to be invoked during normal usage.

Thanks!

@pauldmccarthy
Copy link
Contributor Author

pauldmccarthy commented Oct 28, 2017

Managed to find 10 minutes free - can confirm that #572 solves the problem

@pauldmccarthy
Copy link
Contributor Author

Thanks all for the very prompt work on this!

@effigies
Copy link
Member

👍 Thanks, @matthew-brett!

Does this warrant a bug-fix release (2.2.1)?

@matthew-brett
Copy link
Member

No objection from my side - @pauldmccarthy - do you need a new release with this?

@pauldmccarthy
Copy link
Contributor Author

@effigies and @matthew-brett No, I don't need this to be officially released, but have no objections!

@effigies
Copy link
Member

Okay. Well, if nobody's clamoring for the release, then I think we can hold off. We were talking of another release in a couple weeks which can include these fixes, and (hopefully) image slicing.

@sxg
Copy link

sxg commented Nov 22, 2017

I'd very much appreciate a new release for this fix if possible. I can wait if a bigger release is just a week or so away.

@effigies
Copy link
Member

Hi @sxg, I think we can push out a bug fix release by next week at the latest. The other changes I was hoping to get in on a short time frame have been delayed by life.

@matthew-brett Are you okay with releasing given the Windows issues for Py3.4 on AppVeyor, or is that something that needs resolving first?

@matthew-brett
Copy link
Member

Chris - it turns out we can fix the Appveyor failures easily by dropping conda - https://ci.appveyor.com/project/matthew-brett/nibabel/build/1.0.1/job/imvybv1kbyud87dq

I'll put a PR up tomorrow.

@matthew-brett
Copy link
Member

Looks like the Appveyor fix is working - fine to release from my point of view.

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

Successfully merging a pull request may close this issue.

5 participants