-
-
Notifications
You must be signed in to change notification settings - Fork 15.5k
nbstripout: init at 0.3.0 #25609
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
Conversation
Nope, this is typically the preferred way!
This is indeed odd. So, I see that in the I suggest you wipe this line, e.g. with |
# 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" '' |
There was a problem hiding this comment.
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.
92ec46a
to
f6fcde6
Compare
I used Also, I removed
|
Very strange. I can reproduce the issue but have no idea what causes it. |
It is related to |
@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 In any case, you shouldn't add |
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 |
You could give that a try. Frankly, I would just skip the tests in this case, since we should simply support having mercurial as |
I managed to get it working on Python 2.7. Because this is basically an application, not a library, I moved it away from |
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. |
Is this ok or should I fix something? |
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. |
I got an email from Hydra that job nixpkgs:trunk:nbstripout.i686-linux has failed to build.
So, |
That's up to you if you want to support i686 or not. I wouldn't bother with it and just add
|
Any ideas what could be wrong here, I'm pretty sure it's related to NixOS:
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:
it works correctly. Also, if I just run Some more details here: kynan/nbstripout#55 |
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') |
What do you mean, "with Python 3", this PR added the |
I think (not sure though) I should make |
Could you share with me a branch.
…On Thu, May 18, 2017 at 8:28 PM, Jaakko Luttinen ***@***.***> wrote:
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.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#25609 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ACB8787x0WlBafADj60u5ZzsjhmSG4sHks5r7I3MgaJpZM4NTlMW>
.
|
I just changed Python 2 to Python 3: https://github.com/jluttine/nixpkgs/tree/add-nbstripout The piping test fails in that case. |
I'll have a look at it. It may be relevant that on Python 2.7 we use
ipython 5.3 and on 3.5 we use 6.0.
…On Thu, May 18, 2017 at 8:40 PM, Jaakko Luttinen ***@***.***> wrote:
I just changed Python 2 to Python 3: https://github.com/jluttine/
nixpkgs/tree/add-nbstripout
The piping test fails in that case.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#25609 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ACB8793jnecl7duAea36omGtpet_9QeCks5r7JC4gaJpZM4NTlMW>
.
|
Using IPython 5.3 with Python 3.5 doesn't solve the issue. I tried removing |
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:
Why is it trying to download
pytest-runner
witheasy_install
as the package is already defined inbuildInputs
? I looked at other packages that usepytestrunner
as a build input and they don't seem to do anything special, just addpytestrunner
as a build input.Things done
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"
./result/bin/
)