Skip to content

Conversation

@mulhod
Copy link
Contributor

@mulhod mulhod commented Nov 9, 2020

No description provided.

@pep8speaks
Copy link

pep8speaks commented Nov 9, 2020

Hello @mulhod! Thanks for updating this PR.

Line 70:101: E501 line too long (134 > 100 characters)

Comment last updated at 2020-11-09 21:27:43 UTC

@codecov
Copy link

codecov bot commented Nov 9, 2020

Codecov Report

Merging #634 (d5d16f6) into main (7a7794f) will increase coverage by 0.00%.
The diff coverage is 78.20%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #634   +/-   ##
=======================================
  Coverage   95.10%   95.11%           
=======================================
  Files          27       27           
  Lines        3087     3089    +2     
=======================================
+ Hits         2936     2938    +2     
  Misses        151      151           
Impacted Files Coverage Δ
skll/metrics.py 97.87% <ø> (ø)
skll/utils/commandline/skll_convert.py 93.05% <0.00%> (ø)
skll/data/readers.py 90.13% <47.05%> (ø)
skll/learner/__init__.py 96.26% <55.55%> (ø)
skll/learner/utils.py 94.94% <66.66%> (ø)
skll/data/writers.py 94.11% <73.07%> (+0.09%) ⬆️
skll/experiments/utils.py 93.45% <75.00%> (-0.07%) ⬇️
...utils/commandline/compute_eval_from_predictions.py 97.18% <75.00%> (ø)
skll/experiments/__init__.py 95.19% <84.21%> (ø)
skll/config/__init__.py 95.09% <85.71%> (ø)
... and 11 more

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 7a7794f...d5d16f6. Read the comment docs.

@mulhod
Copy link
Contributor Author

mulhod commented Nov 9, 2020

According to the codecov diff, coverage increases very slightly by 0.01%. So, why the codecov check itself is not passing I'm not really sure.

@desilinguist
Copy link
Collaborator

@mulhod can you push another commit?

@mulhod
Copy link
Contributor Author

mulhod commented Nov 9, 2020

Hmm, codecov check still failed.

@mulhod
Copy link
Contributor Author

mulhod commented Nov 9, 2020

Or perhaps I'm just reading it wrong? The codecov/patch check failed. But not codecov/project.

Anyway something failed.

@desilinguist
Copy link
Collaborator

Yeah, that's the patch diff that failed – which checks whether any new lines you added are also covered by tests. I am guessing these are because you broke some long strings into smaller ones?

@mulhod
Copy link
Contributor Author

mulhod commented Nov 9, 2020

I did break some lines up, but it appears the total didn't change by much. Maybe 2 lines more?

Anyway, it's a little unclear to me how this can be addressed or if it should.

@desilinguist
Copy link
Collaborator

I don't think we need to worry about it.

Copy link
Collaborator

@desilinguist desilinguist left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏽 !

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I have one small suggestion, else it LGTM.

@desilinguist desilinguist merged commit 3dafc5f into main Nov 9, 2020
@delete-merged-branch delete-merged-branch bot deleted the fstrings branch November 9, 2020 21:52
@desilinguist desilinguist mentioned this pull request Nov 9, 2020
@mulhod mulhod linked an issue Nov 16, 2020 that may be closed by this pull request
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.

Use f-strings

4 participants