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

Further improvements to DAG handling #73

Merged
merged 5 commits into from Feb 4, 2019

Conversation

Projects
None yet
2 participants
@charlesreid1
Copy link
Contributor

charlesreid1 commented Jan 31, 2019

Pull request to expand #69 to handle three directed acyclic graph flags:

  • --dag - prints workflow dag in dot format to stdout
  • --dagfile=<dotfile> - prints workflow dag in dot format to a dot file
  • --dagpng=<pngfile> - uses dot to turn the graphviz dot output from snakemake into a png

These flags all turn on the --dry-run flag.

To accomplish the second two flags, I implemented a context manager that will capture stdout when the user is creating the directed acyclic graph, and either prints that captured output to the screen (if the user passes the --dag flag, keeping that flag working as @bluegenes implemented it in #69), or prints the captured output to a file (if the user passes the --dagfile flag), or uses dot to pass the captured output to dot and convert it into a png file (if the user passes the --dagpng flag).

add flags --dagfile and --dagpng
This wraps the Snakemake API call in a
custom context manager class that checks
if we are printing a DAG, and if we are,
when we enter the context, sets up stdout
to be a file buffer, and assembles the
buffer contents into a list when done.

@charlesreid1 charlesreid1 changed the base branch from master to better_dag_handling Jan 31, 2019

@charlesreid1

This comment has been minimized.

Copy link
Contributor Author

charlesreid1 commented Jan 31, 2019

Example commands:

./run_eelpond examples/nema.yaml default --dag > dag.dot
./run_eelpond examples/nema.yaml default ---dagfile dag.dot
./run_eelpond examples/nema.yaml default --dagpng dag.png
@charlesreid1

This comment has been minimized.

Copy link
Contributor Author

charlesreid1 commented Jan 31, 2019

TODO: add tests

charlesreid1 added some commits Jan 31, 2019

@charlesreid1 charlesreid1 force-pushed the cmr_better_dag_handling branch from 78153d9 to aaa7f63 Jan 31, 2019

@charlesreid1

This comment has been minimized.

Copy link
Contributor Author

charlesreid1 commented Jan 31, 2019

Oh, and hey, look at that - a blog post from May 2014 on context managers, which we used in this pull request, from @camillescott - http://www.camillescott.org/2014/05/05/context-management/

@bluegenes
Copy link
Collaborator

bluegenes left a comment

Thanks Charles!!! This looks awesome and definitely nicer to use. What do you think about putting the CaptureStdout class in a file in the ep_utils folder to keep things cleaner within run_eelpond?

@charlesreid1

This comment has been minimized.

Copy link
Contributor Author

charlesreid1 commented Jan 31, 2019

Good idea! I went ahead and made those changes.

@charlesreid1

This comment has been minimized.

Copy link
Contributor Author

charlesreid1 commented Jan 31, 2019

Before I implement a test for this, I just wanted to make some notes here about the eelpond testing approach.

I just noticed the tests you're running are in the .travis.yml file but not in a script or a test directory. Thinking about this CaptureStdout object, it would actually allow you to do something really cool - you can wrap the tests you are currently running from the shell (via .travis.yml) in a python script (a unit test) that could be run with pytest. In other words, if you want to run a shell command like ./run_eelpond arg1 arg2 you could write a unit test that calls that command from Python using subprocess.call(['./run_eelpond','arg1','arg2']).

If you then use the context manager approach to capture stdout from that subprocess call that would allow capturing the output from the Snakemake test (edit: on second thought you may be able to do this with subprocess's PIPE and not need to bother with the context manager) - and check it for specific text, rather than simply checking that it doesn't crash. (This is useful if you have a workflow that may go several different routes, and you want to not just verify the workflow is running but make sure it is following the right logical path).

I'll put together a unit test for the new --dag flags shortly - everything mentioned above will go in a different pull request - but just wanted to to jot down these thoughts somewhere.

@charlesreid1 charlesreid1 force-pushed the cmr_better_dag_handling branch from 12afda9 to b73270d Feb 1, 2019

Add eelpond tests/ directory (#75)
* Add eelpond tests/ directory

This adds individual unit tests for
each of eelpond's flags.

Not all of these flag unit tests are
implemented yet, but the --dag and
--dagfile flags are tested, as are
the --help and --dry-run flags,
to demonstrate how to do it.

* fill in --dagpng test. clean up test_flags test

* add pytest to environment.yml, add pytest command to travis

@bluegenes bluegenes merged commit f71c9bd into better_dag_handling Feb 4, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@bluegenes bluegenes deleted the cmr_better_dag_handling branch Feb 11, 2019

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