-
Notifications
You must be signed in to change notification settings - Fork 192
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
nf-core Launch web GUI #638
Conversation
Moved code into nf_core.utils as reusable functions, so that they can be used for other stuff aside from nf-core schema build Also means some reduction in code duplication
* Use pre-release version of PyInquirer - capture keyboard interruptions, set defaults for lists * Make booleans use a True / False select list instead of confirm prompt * Various tweaks and changes / updates to make launch work with new web launch GUI
* Fix for schema website response type * Fix for launch web filtering for booleans
setup.py
Outdated
'PyInquirer', | ||
# 'PyInquirer>1.0.3', | ||
# Need the new release of PyInquirer, see nf_core/launch.py for details | ||
'PyInquirer @ git+ssh://git@github.com/CITGuru/PyInquirer.git@master#egg=PyInquirer', |
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.
Just like you I am working with PyInquirer and need the latest release.
Do you think that the solution here is suitable for PyPI?
pypa/pip#4187 (comment)
Or are you for now just hoping that there will be a new release soonish?
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.
All right.
So I will go with https://github.com/tmbo/questionary .
Seems to be much more up to date (tech wise) and more actively maintained.
Looks like it supports pretty much the same stuff as PyInquirer.
Might be of interest to you?
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.
Yes I've looked over questionary, there was another one mentioned in the threads too. I wasn't too keen on rewriting the code for a different library though, especially when the two features I needed are already merged into PyInquirer. I'm hoping that he does a new minor release soon. If we get close to doing a new release of nf-core/tools and it's still not out then I'll come back to this and look into questionary.
It's kind of ironic, as apparently PyInquirer was forked from another previous project because of the lack of updates 😅
Codecov Report
@@ Coverage Diff @@
## dev #638 +/- ##
==========================================
+ Coverage 75.45% 76.02% +0.57%
==========================================
Files 11 11
Lines 2159 2269 +110
==========================================
+ Hits 1629 1725 +96
- Misses 530 544 +14
Continue to review full report at Codecov.
|
I've tested the Launch web GUI and it looks really nice and it's very intuitive and easy to use! Great job! |
Brilliant - thanks @ggabernet! 😊 To confirm, did you combine it with running the command line tool, and it worked? |
Yes, it worked! I installed your latest version of the launch-web-gui branch, and tested it with the |
Ah, however using only the
I guess because of the PyInquirer issue? |
Yup - if you re-run the installation ( |
To clarify - I guess that you installed this when you first tested, then just pulled the updated code? If that's the case you'll need to rerun the installation as in my comment above. If you're installing like that and you're still getting that error then it's potentially an installation error problem I guess? |
Hi Phil, I initially tested it by installing your branch of nf-core tools directly from git with:
Now, I've also tested it with cloning your fork, checking out to the
That is the log obtained from the installation, in case it helps for debugging:
So it seems that the installation was successful. But I got the same error when using the
|
It is slightly troubling in the way that PyInquirer is breaking stuff here... |
Hmm, ok - so
I'm pretty sure that this is only an issue because you had already installed PyInquirer, should hopefully work ok in fresh installs. |
@drpatelh yes - I'm hoping that there will be a new PyPI release before we push this new version of tools. See CITGuru/PyInquirer#90 (comment) If we get close to a release and this is the only thing missing then I'll downgrade the code so that it works with the existing latest version - it's only a couple of minor features that I'm using here. It'd be a shame, but it's doable. Alternatively I could rewrite to use a different library if needed. I'll make an issue about bumping this version and add it to the release milestone so that we don't forget. |
@ewels yes, this solved the issue, thanks! |
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.
Not that I can understand completely what is going on in the code, but the functional tests looked good to me!
Great! The |
New code for using a nice web graphical user interface when running
nf-core launch
. Should work with any nextflow pipeline.Bunch of other small tweaks and improvements to
nf-core launch
andnf-core schema
too.Tests haven't been updated so will probably get a lot of failures. Will fix these next week. In the mean time, any code review and / or testing the tools are very welcome!
Works in tandem with changes to the nf-core website, see nf-core/website#400
See Slack
#json-schema
for screen capture demos: https://nfcore.slack.com/archives/CUC8PPDL2/p1593212486143900PR checklist
docs
is updatedCHANGELOG.md
is updatedREADME.md
is updated