I think the drop in coverage is because previous tests never included any of visualize.py. I noticed that coveralls claims that is a new file (with relatively small coverage). @jhprinz : is visualize supposed to be ignored in unit tests? (it's a bit hard to properly test it, and it currently requires separate imports than the OPS namespace)

@SarahMcCartan, @stefanopoggio, @mackernan : I think this can be solved by simply removing the line import openpathsampling.visualize as vis (line 4) from shifting.py. It looks like you don't actually use that import in the rest of the file, so it shouldn't cause any problems. It would still remain in your example notebook. (You should check for other unnecessary imports; I'm pretty sure the IPython.display import in line 5 is also extraneous.)

Hello @SarahMcCartan, @stefanopoggio , @mackernan Great job, looks ready to merge. I made a single comment about the shape of the .detail object, but that is almost philsophical...

Current error is in the build process; that's on us to fix.

@jhprinz : I think that removing all pins in #677 is causing problem here. Error is during build: https://travis-ci.org/openpathsampling/openpathsampling/builds/218806861#L948

I was a similar error in the sphinx build of today's nightly. Pinning hdf4=4.2.12 (maybe jpeg=8d-2 as well?) should fix it. I'm also having build errors in my downstream projects that depend on OPS because of it.

I just opened #681; I hope that will fix it.

Merged #681. Can you merge the current master and try again?

@SarahMcCartan : Just pinging, that you should be able to merge the fix #681 now. After that we can merge!

@jhprinz : We should keep ESDW contribs "on hold" until the paper is accepted. But while it's still fresh in their minds, let's get it to the point that it is ready to be merged! That way we don't need to bug them about cleanup later.

@SarahMcCartan (or @stefanopoggio or @mackernan): You may still need to set the main OPS repo as a remote. Follow the instructions here: https://help.github.com/articles/configuring-a-remote-for-a-fork/ (where both ORIGINAL_OWNER and ORIGINAL_REPOSITORY are replaced by openpathsampling). (If you already see upstream in the results from step 1, then you've already done this.) After that you'll need to bring the current OPS code (which includes the fix from #681) into your branch, and then tell github that you have included those changes. The first part can be done with git fetch upstream && git merge upstream/master; the second with git push. (Make sure you're in the clone of the repo that is based on @SarahMcCartan's fork, and that you're in the shifting branch!)

Hi @jhprinz and @dwhswenson thanks for your messages and clear instructions, apologies I am only seeing this now. I will action this in the morning first thing!

Hi @dwhswenson and @jhprinz, looks like all ok with latest pull request. If we need to do anything else please let us know!
(fyi @stefanopoggio , @mackernan )

Looks so -- I'll pass through it with a proper review later, which might turn up some minor cleanup (mostly Python PEP8 style stuff, I suspect).

Marking this as "on hold," as with other stuff that is complete but waiting for version 1.1.

Please note that we have changed to the OPS license from LGPL (2.1 or later) to MIT. For any pull request to OPS that was started while the license was still LGPL, I need an explicit confirmation that you approve of the license change. Please add a comment with something like "The changes in this pull request are licensed under the MIT license."