-
Notifications
You must be signed in to change notification settings - Fork 79
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
qiita_test_install #805
qiita_test_install #805
Conversation
option_parser, opts, args = parse_command_line_parameters(**script_info) | ||
|
||
if opts.haiku: | ||
print "Qiita provides insight\nmicrobial in nature\nto ecology" |
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.
6-7-5, should be 5-7-5 🗼
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.
A haiku doesn't actually have an enforced syllabic structure, but the
abused English form does. It is 5-7-5 though for that form though
On Jan 20, 2015 9:21 PM, "Joshua Shorenstein" notifications@github.com
wrote:
In scripts/qiita_test_install
#805 (diff):
- test.assertTrue(exists(fp), "%s set to an invalid file path: %s" %
(variable, fp))
- modes = {R_OK: "readable",
W_OK: "writable",
X_OK: "executable"}
test if file readable
- test.assertTrue(access(fp, access_var),
"%s is not %s: %s" % (variable, modes[access_var], fp))
+def main():
- option_parser, opts, args = parse_command_line_parameters(**script_info)
- if opts.haiku:
print "Qiita provides insight\nmicrobial in nature\nto ecology"
6-7-6, should be 5-7-5 [image: 🗼]
—
Reply to this email directly or view it on GitHub
https://github.com/biocore/qiita/pull/805/files#r23252284.
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.
oh noes, the secret feature!
On (Jan-20-15|21:28), Daniel McDonald wrote:
- test.assertTrue(exists(fp), "%s set to an invalid file path: %s" %
(variable, fp))
- modes = {R_OK: "readable",
W_OK: "writable",
X_OK: "executable"}
test if file readable
- test.assertTrue(access(fp, access_var),
"%s is not %s: %s" % (variable, modes[access_var], fp))
+def main():
- option_parser, opts, args = parse_command_line_parameters(**script_info)
- if opts.haiku:
print "Qiita provides insight\nmicrobial in nature\nto ecology"
A haiku doesn't actually have an enforced syllabic structure, but the
abused English form does. It is 5-7-5 though for that form though
On Jan 20, 2015 9:21 PM, "Joshua Shorenstein" notifications@github.com
wrote:In scripts/qiita_test_install
#805 (diff):
- test.assertTrue(exists(fp), "%s set to an invalid file path: %s" %
(variable, fp))
- modes = {R_OK: "readable",
W_OK: "writable",
X_OK: "executable"}
test if file readable
- test.assertTrue(access(fp, access_var),
"%s is not %s: %s" % (variable, modes[access_var], fp))
+def main():
- option_parser, opts, args = parse_command_line_parameters(**script_info)
- if opts.haiku:
print "Qiita provides insight\nmicrobial in nature\nto ecology"
6-7-6, should be 5-7-5 [image: 🗼]
—
Reply to this email directly or view it on GitHub
https://github.com/biocore/qiita/pull/805/files#r23252284.
Reply to this email directly or view it on GitHub:
https://github.com/biocore/qiita/pull/805/files#r23278780
Would be nice to
|
This is ready for review. |
# we need to run the test suite from setup.py for coveralls to grab the info | ||
# - coverage run setup.py test | ||
# - coverage report -m | ||
- qiita_test_install |
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.
Should this be moved up to second thing called in the script? That way we have all the info before tests are run, so we can fix things as they error if needed.
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.
Could be, no preference.
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.
Probably better, since if anything fails we won't have that information for diagnosis.
Traceback (most recent call last): |
How do I reproduce that? |
I just ran it and that happened. |
Which version of moi are you using? |
Lets use the script :)
|
print "%*s:\t%s" % (max_len, v[0], v[1]) | ||
|
||
qiita_config = ConfigurationManager() | ||
try: |
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.
This needs an exists test after the try-except to make sure the file actually exists at the path given
else: | ||
ipython_running = False | ||
except e: | ||
print e |
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.
This needs something better here than just a print of the error, for user information sake. This is what I am getting with the paste in the main discussion thread.
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.
Agree, that was left by mistake.
@squirrelo @ElDeveloper any other comments? Is this ready for merge? |
ipython_running = True | ||
else: | ||
ipython_running = False | ||
except e: |
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.
Line 365 is still thorwing a traceback and not showing the rest on my computer. Perhaps this should be
except Exception as e:
or just
except:
since we don't actually need e.
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.
Good point, done!
missing_deps_errors = [] | ||
missing_deps_warnings = [] | ||
|
||
try: |
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.
I know exec is usually frowned upon, but what about replacing these few lines with something like this:
for dependency in ['wtforms', 'networkx']:
try:
exec 'from {0} import __version__ as {0}_lib_version'.format(dependency)
except ImportError:
missing_deps_warnings.append((e, 'Some message'))
exec "{0}_lib_version = 'Not Installed'"
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.
what about using importlib
?
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.
That would have also worked, though note that this PR is now merged, so
we may want to open a separate issue about this.
On (Feb-03-15|13:06), Daniel McDonald wrote:
- 'Qiita Installation Guide: '
- 'https://github.com/biocore/qiita/blob/master/INSTALL.md')
+extra_info = (core_dependency_missing_msg + '. It is also possible that you '
'have an old version of this package, if so, please update to '
'the latest.')
+dependency_missing_msg = (
- '"%s" is missing but this is not a core requirement, for more '
- 'information see the Qiita Installation Guide: '
- 'https://github.com/biocore/qiita/blob/master/INSTALL.md')
+missing_deps_errors = []
+missing_deps_warnings = []
+
+try:what about using
importlib
?
Reply to this email directly or view it on GitHub:
https://github.com/biocore/qiita/pull/805/files#r24039719
Oh ... ok ;l |
Initial commit for discussion during meeting.