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

Paths ported from jupyter_core #19

Merged
merged 11 commits into from
May 23, 2015
Merged

Conversation

rgbkrk
Copy link
Member

@rgbkrk rgbkrk commented May 22, 2015

This is a port of https://github.com/jupyter/jupyter_core while also including ~/.ipython/ as part of the path search. This ensures that the default directories that the Jupyter consoles and notebook searches through are the same for Hydrogen.

The core function here is jupyterPath, which gets used like so:

coffee> jupyterPath('kernels')
[ '/Users/kyle6475/Library/Jupyter/kernels',
  '/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/share/jupyter/kernels',
  '/usr/local/share/jupyter/kernels',
  '/usr/share/jupyter/kernels',
  '/Users/kyle6475/.ipython/kernels' ]
coffee> jupyterPath('runtime')
[ '/Users/kyle6475/Library/Jupyter/runtime',
  '/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/share/jupyter/runtime',
  '/usr/local/share/jupyter/runtime',
  '/usr/share/jupyter/runtime',
  '/Users/kyle6475/.ipython/runtime' ]

This will also be useful for picking up what running kernels are available for the notebook, another console (use Hydrogen on the side of a console session!), etc.

@rgbkrk
Copy link
Member Author

rgbkrk commented May 22, 2015

Weird, having trouble getting a kernel to start even off master.

Totally my fault. There was a problem on a branch I was running in dev mode that the IPython kernel was depending on. Working well. That's what I get for straddling two projects at once.

screenshot 2015-05-22 13 16 25

@rgbkrk rgbkrk changed the title [WIP] Paths ported from jupyter_core Paths ported from jupyter_core May 22, 2015
@rgbkrk
Copy link
Member Author

rgbkrk commented May 22, 2015

I'd also be open to making a package like this that can be used across packages, as I'd use this in side car.

@rgbkrk
Copy link
Member Author

rgbkrk commented May 22, 2015

I'm starting to add tests. In the process I tore out the boilerplate Atom specs for a new package (caused failures). Once this is done, it would be nice to enable Travis CI.

else
return path.join jupyterConfigDir(), 'data'

home = userHome()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's make this an if... else if... else instead of relying on the returns to be more explicit about the control flow

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@willwhitney
Copy link
Collaborator

This looks good, and it runs great for me! Have you been able to test it on Windows?

@rgbkrk
Copy link
Member Author

rgbkrk commented May 23, 2015

Haven't been able to test this on Windows. I did a minimal amount of adding some mocking specs in (while tossing the skeleton code), but that doesn't say for sure how Windows goes.

@willwhitney
Copy link
Collaborator

Given that our Windows support is already weak at best, I'll just ship it and see how it goes.

willwhitney added a commit that referenced this pull request May 23, 2015
Paths ported from jupyter_core
@willwhitney willwhitney merged commit 8e209ed into nteract:master May 23, 2015
@willwhitney
Copy link
Collaborator

@jzthree Just published this live. I know you've got Hydrogen running on Windows, so please let me know if this broke anything for you!

@jzthree
Copy link
Contributor

jzthree commented May 23, 2015

Hey I just tested. Unfortunately this broke Hydrogen:Run on Windows. The execSync call in paths.coffee failed. I pasted the first few lines of error message here.

Error: EBADF: bad file descriptor, write
at Error (native)
at Object.fs.writeSync (fs.js:657:20)
at SyncWriteStream.write (fs.js:1906:6)
at execSync (child_process.js:1372:20)
at Object. (C:\Users\Jian.WIN-MSIJZ.atom\packages\Hydrogen\lib\paths.coffee:10:12)
at Object. (C:\Users\Jian.WIN-MSIJZ.atom\packages\Hydrogen\lib\paths.coffee:3:1)

@rgbkrk rgbkrk deleted the path_ported branch May 23, 2015 13:13
@rgbkrk
Copy link
Member Author

rgbkrk commented May 23, 2015

Well now I wish I had a Windows box quite a bit... I'm surprised readSync doesn't work but perhaps it's not finding python on the path.

@rgbkrk
Copy link
Member Author

rgbkrk commented May 23, 2015

Hey @zooba, any ideas?

@willwhitney
Copy link
Collaborator

@jzthree Just pushed an update that replaces execSync calls with spawnSync, which seems to not cause this "bad file descriptor, write" error in my tests. Mind running it when you get a chance?

@jzthree
Copy link
Contributor

jzthree commented May 24, 2015

Yes just updated and it all works now. Thanks!!

@rgbkrk
Copy link
Member Author

rgbkrk commented May 24, 2015

Thank you @willwhitney, sorry about that.

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 this pull request may close these issues.

None yet

4 participants