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

A few bugfixes #19

Merged
merged 3 commits into from Mar 19, 2018
Merged

A few bugfixes #19

merged 3 commits into from Mar 19, 2018

Conversation

remiolsen
Copy link
Member

Some changes I made to make linting work on my python 3.6 installation

@codecov-io
Copy link

codecov-io commented Mar 16, 2018

Codecov Report

Merging #19 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #19      +/-   ##
==========================================
+ Coverage   98.02%   98.03%   +0.01%     
==========================================
  Files           2        2              
  Lines         152      153       +1     
==========================================
+ Hits          149      150       +1     
  Misses          3        3
Impacted Files Coverage Δ
nf_core/lint.py 98% <100%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f31b244...589ca66. Read the comment docs.

nf_core/lint.py Outdated
@@ -104,7 +104,7 @@ def pf(file_path):
files = [files]
if any([os.path.isfile(pf(f)) for f in files]):
self.passed.append((1, "File found: {}".format(files)))
self.files.append(f)
self.files.append(files)
Copy link
Member

Choose a reason for hiding this comment

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

I think it should be extend(files), as files will be a list.

nf_core/lint.py Outdated
@@ -114,7 +114,7 @@ def pf(file_path):
files = [files]
if any([os.path.isfile(pf(f)) for f in files]):
self.passed.append((1, "File found: {}".format(files)))
self.files.append(f)
self.files.append(files)
Copy link
Member

Choose a reason for hiding this comment

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

same here

nf_core/lint.py Outdated
@@ -269,7 +269,7 @@ def check_readme(self):
nf_config_version = self.config.get('params.nf_required_version').strip('\'"')
try:
assert nf_badge_version == nf_config_version
except AssertionError, KeyError:
except (AssertionError, KeyError) as e:
Copy link
Member

Choose a reason for hiding this comment

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

not necessary imo, cause the exception object is not accessed anywhere

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right. I guess writing as e is more of a reflex for me 😄

@sven1103
Copy link
Member

Thanks Remi! Could you also explain what you mean with "work on your Python 3.6" installation? Cause we are running CI with several Python versions. So if something is not working as expected, would be good to know (or even raise an issue) :)

Copy link
Member

@sven1103 sven1103 left a comment

Choose a reason for hiding this comment

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

nice

Copy link
Member

@sven1103 sven1103 left a comment

Choose a reason for hiding this comment

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

sry, need to resolve the list thing first

nf_core/lint.py Outdated
@@ -210,7 +210,8 @@ def check_config_vars(self):
raise AssertionError("`nextflow config` returned non-zero error code: %s,\n %s", e.returncode, e.output)
else:
for l in nfconfig_raw.splitlines():
k, v = str(l).split(' = ', 1)
ul = l.decode()
k, v = ul.split(' = ', 1)
Copy link
Member

Choose a reason for hiding this comment

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

nice!

@remiolsen
Copy link
Member Author

remiolsen commented Mar 19, 2018

Hi Sven. Thanks. I was testing just testing this tool on my Nextflow pipeline and using the version of python I have installed locally (Python 3.6.3 :: Anaconda, Inc.). It would throw two exceptions at various stages. SyntaxError on the exception block. And also another when making comparisons between byte-literals a unicode string.

@sven1103
Copy link
Member

Would you mind opening an issue and describe this in detail (StackTrace etc.) Have to look into this, because the CI does not throw the exceptions :(

@remiolsen
Copy link
Member Author

No worries, Sven. See #20

@sven1103 sven1103 merged commit a3cacd3 into nf-core:master Mar 19, 2018
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.

None yet

3 participants