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

pydoit compatibility for nbflow #5

Open
wants to merge 14 commits into
base: master
from

Conversation

@camillescott
Copy link
Contributor

@camillescott camillescott commented Jul 15, 2016

Hello there!

I like to use pydoit as my build system; it supports Python3 and is quite extensible. This PR adds a module to nbflow for pydoit.

The folder doit-example shows how it works. It uses a dodo.py file, which follows a similar pattern to the scons example:

from nbflow.doit import create_build_tasks

def task_nbflow():
    yield from create_build_tasks(['analyses'])

Major kudos for your arch making this really easy to write!

Todos:

  • Needs tests ;)
  • Works only with Python 3, which conflicts with he scons module and example notebooks. See here for instructions on putting Python 2 and 3 kernels side by side.
  • Should possibly be its own package, something like nbflow-doit
  • A couple bugs have been fixed which I need to push downstream (more PRs incoming)
  • If keeping in same project, update README

Cheers,
Camille

@jhamrick
Copy link
Owner

@jhamrick jhamrick commented Jul 15, 2016

Thanks! Let's loop @joestubbs too who was thinking of working on exactly this as well.

@@ -0,0 +1,7 @@
# Example nbflow setup

This comment has been minimized.

@jhamrick

jhamrick Jul 15, 2016
Owner

I would prefer if we actually kept just a single example that can be run multiple ways -- so if we have the SConstruct and dodo.py side-by-side and users can either run scons or doit depending on which system they want to try. Then we also don't need to maintain multiple almost identical examples.

This comment has been minimized.

@camillescott

camillescott Jul 16, 2016
Author Contributor

Good point; done.

yield from touch_cmds(target)


def create_build_tasks(directories):

This comment has been minimized.

@jhamrick

jhamrick Jul 15, 2016
Owner

Let's call this setup to mirror the API for scons

@@ -57,7 +57,7 @@ def get_dependencies(self, dirnames):
sources = [self.resolve_path(filename, x) for x in params['__depends__']]

targets = params['__dest__']
if not hasattr(targets, '__iter__'):
if isinstance(targets, str):

This comment has been minimized.

@jhamrick

jhamrick Jul 15, 2016
Owner

What is the motivation behind this change? __dest__ = None is a valid use of nbflow but None is not an instance of str, so this will not have the same behavior as before.

This comment has been minimized.

@camillescott

camillescott Jul 16, 2016
Author Contributor

Strings in Python 3 have an __iter__ method, which was causing some rather interesting erroneous targets. But now I see that you're right, this version breaks on None instead. Need to check for that.

@@ -1,4 +1,5 @@
traitlets
nbconvert==4.1
scons

This comment has been minimized.

@jhamrick

jhamrick Jul 15, 2016
Owner

If we're going to be having multiple build systems supported, I would actually not make any of them a dependency -- so rather than adding doit as a dependency, remove scons as a dependency.

@joestubbs
Copy link

@joestubbs joestubbs commented Jul 15, 2016

Thanks for looping me in! I wrote a pydoit.py module with a setup function as we discussed, and the original workflow example builds successfully, but I hadn't added any additional tests. My implementation doesn't add pydoit to the requirements; rather, it assumes the user has installed it themselves and has their own dodo.py file (or task loader) that they can complement with the setup function.

If this would be of use I'm happy to make it available - just let me know.

@ajasja
Copy link

@ajasja ajasja commented Aug 8, 2016

Hi!
Just watched the Scipy 2016 talk. Great Idea!
I would also much prefer to use pydoit instead of Scons.
Best,
Ajasja

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

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.