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

ENH: Add feedback to result saving #110

Merged
merged 2 commits into from
Dec 15, 2016
Merged

Conversation

jakereps
Copy link
Member

@jakereps jakereps commented Dec 15, 2016

Resolves #98

Thoughts on verbiage? Saved to: Written to: <filepath only>. Extend the filepath beyond the relative path that is returned by .save()?

(qiime2) [17:09] qiime2-moving-pictures-tutorial $ qiime diversity beta-group-significance --i-distance-matrix cm1441/unweighted_unifrac_distance_matrix.qza --m-metadata-file sample-metadata.tsv --m-metadata-category BodySite --o-visualization cm1441/unweighted-unifrac-body-site-significance
Saved to: cm1441/unweighted-unifrac-body-site-significance.qzv
(qiime2) [17:09] qiime2-moving-pictures-tutorial $ 

@jairideout
Copy link
Member

Thoughts on verbiage?

👍, I like the current wording. Do you think it'd be helpful to distinguish between artifacts and visualizations? i.e. Saved artifact to: foo/bar.qza and Saved visualization to: baz.qzv

What do you think about echoing using green text? That way it's easy to spot, plus it's a "successful" operation.

Do you think this should always echo (like it is now)? Or should --verbose activate it?

Extend the filepath beyond the relative path that is returned by .save()?

I like the current behavior because it'll be relative if the user provided a relative path, and absolute otherwise.

@thermokarst @ebolyen, interested in your thoughts.

@thermokarst
Copy link
Contributor

I like differentiating between viz and artifact in the message, also formatting the text green sounds great. I also like the current behavior, echoing whatever the user provided as far as path. Maybe --verbose could expand relative to absolute, and pass through absolute as-is?

@jakereps
Copy link
Member Author

Thanks @jairideout and @thermokarst, here's the updated output:
image

Copy link
Contributor

@thermokarst thermokarst left a comment

Choose a reason for hiding this comment

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

This looks so fancy @jakereps!

@thermokarst thermokarst merged commit 633e950 into qiime2:master Dec 15, 2016
@jairideout
Copy link
Member

👍 , thanks @jakereps !

@thermokarst thermokarst mentioned this pull request Dec 16, 2016
ChrisKeefe pushed a commit to ChrisKeefe/q2cli that referenced this pull request Jan 10, 2019
ENH: Adds qiime.plugin.plugin_init to initialize plugin package from …
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.

3 participants