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

fix: fix flake8 errors in report module #189

Merged
merged 1 commit into from Feb 12, 2021

Conversation

sankha555
Copy link
Collaborator

@codecov
Copy link

codecov bot commented Feb 12, 2021

Codecov Report

Merging #189 (aa9cc0d) into develop (e7f112b) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #189   +/-   ##
========================================
  Coverage    68.46%   68.46%           
========================================
  Files           62       62           
  Lines         4253     4253           
  Branches       721      721           
========================================
  Hits          2912     2912           
  Misses        1113     1113           
  Partials       228      228           
Impacted Files Coverage Δ
src/sbmlutils/report/formating.py 80.86% <100.00%> (ø)
src/sbmlutils/report/sbmlreport.py 76.41% <100.00%> (ø)

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 e7f112b...aa9cc0d. Read the comment docs.

@sankha555
Copy link
Collaborator Author

@matthiaskoenig I have made the changes in module according to the errors generated by the flake8 run. I changed the function summary description in a few places to make it a bit concise.

In some of the strings in sbmlreport.py, where the HTML code fragments are rendered, I have indented those strings into multiline strings since some of them were crossing the character limit per line as indicated by flake8. Would this change be appropriate in terms of standards and readability?

I am a bit confused about what best change I should incorporate for a few instances in the files, which I have asked in the comments in the corresponding line. Please take a look at them and kindly suggest me how I should proceed regarding them. If there are any other changes that I should make, please let me know. Thanks!

@@ -169,12 +169,17 @@ def boundsStringFromReaction(reaction: libsbml.Reaction, model: libsbml.Model) -
if ub_p.isSetValue():
ub_value = ub_p.getValue()
if (lb_value is not None) or (ub_value is not None):
bounds = f'<code>[{lb_value} <i class="fa fa-sort fa-rotate-90" aria-hidden="true"></i> {ub_value}]</code>'
bounds = f"""
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One of the instances, where I have made the HTML code multi-line to overcome the flake8 error

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the first doc string for the files xslt/math.py (line 2) and formating.py(line 2), flake8 says that they should be single line doc-strings. Should I convert them to single line doc strings or shall Iet them be as they are. If we need to update those doc strings in the future to add more detail, then we might have to convert them to multi-line doc strings. Hence I wanted to take your suggestion on what would be best way to go about them.

Copy link
Owner

Choose a reason for hiding this comment

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

xlst will be removed soon, so no updates here. Multi-line comment is fine.

@matthiaskoenig matthiaskoenig merged commit 6260384 into develop Feb 12, 2021
@matthiaskoenig matthiaskoenig deleted the fix-fix-flake8-errors-in-report-module branch February 13, 2021 21:20
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.

Fix flake8 issues in report module
2 participants