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

nbstripout: init at 0.3.0 #25609

Merged
merged 3 commits into from
May 15, 2017
Merged

nbstripout: init at 0.3.0 #25609

merged 3 commits into from
May 15, 2017

Conversation

jluttine
Copy link
Member

@jluttine jluttine commented May 8, 2017

Motivation for this change

nbstripout is a useful filter for git when version controlling Jupyter notebooks.

This pull request also contains the required dependencies as separate commits. Is that ok, or should I open separate pull requests for each commit?

NOTE: This is still work in progress. I'm getting a build error with nox-review. I opened this pull request to get some feedback how to fix that so I can finish this.

When running nox-review, I get the following error:

Copying nbstripout.egg-info to build/bdist.linux-x86_64/wheel/nbstripout-0.3.0-py2.7.egg-info
running install_scripts
creating build/bdist.linux-x86_64/wheel/nbstripout-0.3.0.dist-info/WHEEL
installing
/tmp/nix-build-python2.7-nbstripout-0.3.0.drv-0/nbstripout-0.3.0/dist /tmp/nix-build-python2.7-nbstripout-0.3.0.drv-0/nbstripout-0.3.0
Download error on https://pypi.python.org/simple/pytest-runner/: [Errno -2] Name or service not known -- Some packages may not be found!
Couldn't find index page for 'pytest-runner' (maybe misspelled?)
Download error on https://pypi.python.org/simple/: [Errno -2] Name or service not known -- Some packages may not be found!
No local packages or working download links found for pytest-runner
Traceback (most recent call last):
  File "nix_run_setup.py", line 8, in <module>
    exec(compile(getattr(tokenize, 'open', open)(__file__).read().replace('\\r\\n', '\\n'), __file__, 'exec'))
  File "setup.py", line 52, in <module>
    "Topic :: Software Development :: Version Control",
  File "/nix/store/ngfvnb22lh72z8prslmfdd2aiis5z08p-python3-3.5.3/lib/python3.5/distutils/core.py", line 108, in setup
    _setup_distribution = dist = klass(attrs)
  File "/nix/store/1mbjrzsq7rc6jb4wkcvdjgnqbhcm41jm-python2.7-setuptools-30.2.0/lib/python2.7/site-packages/setuptools-30.2.0-py2.7.egg/setuptools/dist.py", line 315, in __init__
  File "/nix/store/1mbjrzsq7rc6jb4wkcvdjgnqbhcm41jm-python2.7-setuptools-30.2.0/lib/python2.7/site-packages/setuptools-30.2.0-py2.7.egg/setuptools/dist.py", line 361, in fetch_build_eggs
  File "/nix/store/1mbjrzsq7rc6jb4wkcvdjgnqbhcm41jm-python2.7-setuptools-30.2.0/lib/python2.7/site-packages/setuptools-30.2.0-py2.7.egg/pkg_resources/__init__.py", line 846, in resolve
  File "/nix/store/1mbjrzsq7rc6jb4wkcvdjgnqbhcm41jm-python2.7-setuptools-30.2.0/lib/python2.7/site-packages/setuptools-30.2.0-py2.7.egg/pkg_resources/__init__.py", line 1118, in best_match
  File "/nix/store/1mbjrzsq7rc6jb4wkcvdjgnqbhcm41jm-python2.7-setuptools-30.2.0/lib/python2.7/site-packages/setuptools-30.2.0-py2.7.egg/pkg_resources/__init__.py", line 1130, in obtain
  File "/nix/store/1mbjrzsq7rc6jb4wkcvdjgnqbhcm41jm-python2.7-setuptools-30.2.0/lib/python2.7/site-packages/setuptools-30.2.0-py2.7.egg/setuptools/dist.py", line 429, in fetch_build_egg
  File "/nix/store/1mbjrzsq7rc6jb4wkcvdjgnqbhcm41jm-python2.7-setuptools-30.2.0/lib/python2.7/site-packages/setuptools-30.2.0-py2.7.egg/setuptools/command/easy_install.py", line 659, in easy_install
distutils.errors.DistutilsError: Could not find suitable distribution for Requirement.parse('pytest-runner')
builder for ‘/nix/store/frdmmw910nm2hd6mplv3g365q50qy9xx-python3.5-nbstripout-0.3.0.drv’ failed with exit code 1
error: build of ‘/nix/store/frdmmw910nm2hd6mplv3g365q50qy9xx-python3.5-nbstripout-0.3.0.drv’ failed
The invocation of "nix-build -A python27Packages.cram -A python35Packages.cram -A python27Packages.pytest-cram -A python27Packages.nbstripout -A python35Packages.pytest-cram -A python35Packages.nbstripout /etc/nixpkgs" failed

Why is it trying to download pytest-runner with easy_install as the package is already defined in buildInputs? I looked at other packages that use pytestrunner as a build input and they don't seem to do anything special, just add pytestrunner as a build input.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@jluttine, thanks for your PR! By analyzing the history of the files in this pull request, we identified @FRidh to be a potential reviewer.

@FRidh
Copy link
Member

FRidh commented May 8, 2017

This pull request also contains the required dependencies as separate commits. Is that ok, or should I open separate pull requests for each commit?

Nope, this is typically the preferred way!

Why is it trying to download pytest-runner with easy_install as the package is already defined in buildInputs? I looked at other packages that use pytestrunner as a build input and they don't seem to do anything special, just add pytestrunner as a build input.

This is indeed odd. So, I see that in the setup.py they even add it as setup_requires, that's extreme! After a quick look at the code I also don't see why its needed there. In any case, this doesn't explain why it can't find pytest-runner.

I suggest you wipe this line, e.g. with substituteInplace or sed.

# A patch to fix /bin/bash in one unit test. Need to create this patch
# dynamically instead of using static patch file because the patch requires
# the path to bash.
bashPatch = writeText "bash.patch" ''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could use substituteInPlace in the postPatch phase/hook.

@jluttine jluttine force-pushed the add-nbstripout branch 2 times, most recently from 92ec46a to f6fcde6 Compare May 8, 2017 08:25
@jluttine
Copy link
Member Author

jluttine commented May 8, 2017

I used substituteInPlace in a few places.

Also, I removed pytest-runner from setup_requires. However, now I get a new error for nbstripout:

Copying nbstripout.egg-info to build/bdist.linux-x86_64/wheel/nbstripout-0.3.0-py2.7.egg-info
running install_scripts
creating build/bdist.linux-x86_64/wheel/nbstripout-0.3.0.dist-info/WHEEL
installing
/tmp/nix-build-python2.7-nbstripout-0.3.0.drv-0/nbstripout-0.3.0/dist /tmp/nix-build-python2.7-nbstripout-0.3.0.drv-0/nbstripout-0.3.0
usage: nix_run_setup.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
   or: nix_run_setup.py --help [cmd1 cmd2 ...]
   or: nix_run_setup.py --help-commands
   or: nix_run_setup.py cmd --help

error: invalid command 'bdist_wheel'
builder for ‘/nix/store/a69m2jsni183lkg0nlbwfjzx6gqrc8dc-python3.5-nbstripout-0.3.0.drv’ failed with exit code 1
error: build of ‘/nix/store/a69m2jsni183lkg0nlbwfjzx6gqrc8dc-python3.5-nbstripout-0.3.0.drv’ failed
The invocation of "nix-build -A python27Packages.nbstripout -A python35Packages.nbstripout -A python35Packages.pytest-cram -A python27Packages.pytest-cram /etc/nixpkgs" failed

@FRidh
Copy link
Member

FRidh commented May 8, 2017

Very strange. I can reproduce the issue but have no idea what causes it.

@FRidh FRidh self-assigned this May 8, 2017
@jluttine
Copy link
Member Author

jluttine commented May 8, 2017

It is related to mercurial package. It somehow messes the Python environment for Python 3.5.

@FRidh
Copy link
Member

FRidh commented May 8, 2017

@jluttine That is definitely possible. Currently we build Python applications in the same way as library, and we should stop doing that since then, when we add them as buildInput, their PYTHONPATH is taken, as you've now experienced.

In any case, you shouldn't add mercurial as buildInput, but likely you need to add it to the PATH of any script that is generated by wrapping it.

@jluttine
Copy link
Member Author

jluttine commented May 8, 2017

Ok! It's not needed run-time, only in the unit tests. So I wrap the test running into a script with paths to mercurial and git given? That way, they aren't installed for runtime environment unnecessarily because the installed nbstripout script doesn't refer to them?

@FRidh
Copy link
Member

FRidh commented May 8, 2017

You could give that a try. Frankly, I would just skip the tests in this case, since we should simply support having mercurial as buildInput.

@jluttine
Copy link
Member Author

jluttine commented May 8, 2017

I managed to get it working on Python 2.7. Because this is basically an application, not a library, I moved it away from python-modules to application/version-management. nbstripout is a small shell tool that can be used as a filter in git or mercurial to preprocess Jupyter notebooks for version management so I thought this would be a proper place for it.

@jluttine jluttine changed the title pythonPackages.nbstripout: init at 0.3.0 nbstripout: init at 0.3.0 May 8, 2017
@jluttine
Copy link
Member Author

jluttine commented May 8, 2017

On Python 3.5, I got it working otherwise, except some tests failed. I reported that upstream: kynan/nbstripout#55. Not sure whether the fault is in nix or the package itself.

@jluttine
Copy link
Member Author

Is this ok or should I fix something?

@FRidh
Copy link
Member

FRidh commented May 15, 2017

Note that if you use Python 2, you'll be using ipython 5.3, whereas if you use Python 3, you get ipython 6.0.

@FRidh FRidh merged commit 9468764 into NixOS:master May 15, 2017
@jluttine
Copy link
Member Author

I got an email from Hydra that job nixpkgs:trunk:nbstripout.i686-linux has failed to build.
https://hydra.nixos.org/build/52896671/nixlog/4

Below is the build log of derivation /nix/store/ypkfplkaw6dd3vbms3v9jf54iik4flkz-python2.7-cram-0.7.drv. It was built on root@ike.ewi.tudelft.nl.
...
patching script interpreter paths in /nix/store/dyhnjv4ry6f8y375w154lbk1hy172p9q-python2.7-cram-0.7
wrapping `/nix/store/dyhnjv4ry6f8y375w154lbk1hy172p9q-python2.7-cram-0.7/bin/cram'...
running install tests
..s..!
--- tests/interactive.t
+++ tests/interactive.t.err
@@ -277,11 +277,22 @@
      \d (re)
   Accept this change? [yN] y
   patch failed
-  examples/fail.t: merge failed
-  
-  # Ran 1 tests, 0 skipped, 1 failed.
-  [1]
-  $ md5 examples/fail.t examples/fail.t.err
-  .*\b0f598c2b7b8ca5bcb8880e492ff6b452\b.* (re)
-  .*\b7a23dfa85773c77648f619ad0f9df554\b.* (re)
+  Traceback (most recent call last):
+    File "/build/cram-0.7/tests/../scripts/cram", line 7, in <module>
+      sys.exit(cram.main(sys.argv[1:]))
+    File "/nix/store/dyhnjv4ry6f8y375w154lbk1hy172p9q-python2.7-cram-0.7/lib/python2.7/site-packages/cram/_main.py", line 197, in main
+      refout, postout, diff = test()
+    File "/nix/store/dyhnjv4ry6f8y375w154lbk1hy172p9q-python2.7-cram-0.7/lib/python2.7/site-packages/cram/_cli.py", line 121, in testwrapper
+      if _patch(patchcmd, diff):
+    File "/nix/store/dyhnjv4ry6f8y375w154lbk1hy172p9q-python2.7-cram-0.7/lib/python2.7/site-packages/cram/_cli.py", line 54, in _patch
+      out, retcode = execute([cmd, '-p0'], stdin=b('').join(diff))
+    File "/nix/store/dyhnjv4ry6f8y375w154lbk1hy172p9q-python2.7-cram-0.7/lib/python2.7/site-packages/cram/_process.py", line 53, in execute
+      out, err = p.communicate(stdin)
+    File "/nix/store/0rxia0hk1pr3qalfk3j5hq7r20szhm6r-python-2.7.13/lib/python2.7/subprocess.py", line 469, in communicate
+      self.stdin.close()
+  IOError: [Errno 32] Broken pipe
+  [1]
+  $ md5 examples/fail.t examples/fail.t.err
+  0f598c2b7b8ca5bcb8880e492ff6b452  examples/fail.t
+  7a23dfa85773c77648f619ad0f9df554  examples/fail.t.err
   $ rm patch examples/fail.t.err
ss...
# Ran 11 tests, 3 skipped, 1 failed.

builder for ‘/nix/store/ypkfplkaw6dd3vbms3v9jf54iik4flkz-python2.7-cram-0.7.drv’ failed with exit code 1

So, python2.7-cram fails on i686 with BrokenPipe error. I have no idea what could be wrong.. Should I fix this somehow?

@FRidh
Copy link
Member

FRidh commented May 17, 2017

That's up to you if you want to support i686 or not. I wouldn't bother with it and just add

meta.broken = stdenv.isi686;

@jluttine
Copy link
Member Author

Any ideas what could be wrong here, I'm pretty sure it's related to NixOS:

nbstripout can be used with pipes:

cat somefile | nbstripout

However, this only works correctly with Python 2, not Python 3. With Python 3, the output is just empty as if it didn't receive anything from the pipe. However, if I run:

cat somefile | python -m nbstripout

it works correctly. Also, if I just run nbstripout somefile, it processes the file correctly. So it seems like nix does some wrapping to the executable which breaks the piping when running with Python 3. Any ideas?

Some more details here: kynan/nbstripout#55

@jluttine
Copy link
Member Author

nbstripout seem to handle stdin and stdout streams a bit differently in Python 2 and Python 3:

https://github.com/kynan/nbstripout/blob/master/nbstripout.py#L94

if sys.version_info < (3, 0):
    import codecs
    # Use UTF8 reader/writer for stdin/stdout
    # http://stackoverflow.com/a/1169209
    input_stream = codecs.getreader('utf8')(sys.stdin)
    output_stream = codecs.getwriter('utf8')(sys.stdout)
else:
    # Wrap input/output stream in UTF-8 encoded text wrapper
    # https://stackoverflow.com/a/16549381
    input_stream = io.TextIOWrapper(sys.stdin.buffer, encoding='utf-8')
    output_stream = io.TextIOWrapper(sys.stdout.buffer, encoding='utf-8')

@FRidh
Copy link
Member

FRidh commented May 18, 2017

However, this only works correctly with Python 2, not Python 3. With Python 3, the output is just empty as if it didn't receive anything from the pipe. However, if I run:

What do you mean, "with Python 3", this PR added the nbstripout package as an application, running with Python 2.

@jluttine
Copy link
Member Author

I think (not sure though) I should make nbstripout a Python module after all because it depends on IPython so one can easily end up having two different ipython executables (Python 2 and Python 3 variants) in the same environment. I'm just trying to run it with Python 3 now to make it work but it doesn't.

@FRidh
Copy link
Member

FRidh commented May 18, 2017 via email

@jluttine
Copy link
Member Author

I just changed Python 2 to Python 3: https://github.com/jluttine/nixpkgs/tree/add-nbstripout

The piping test fails in that case.

@FRidh
Copy link
Member

FRidh commented May 18, 2017 via email

@FRidh
Copy link
Member

FRidh commented May 18, 2017

Using IPython 5.3 with Python 3.5 doesn't solve the issue. I tried removing mercurial thinking it may be related to the paths mingling somehow, but that didn't make any difference either. I'm out of ideas.

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

Successfully merging this pull request may close these issues.

4 participants