Skip to content

Benchmarks now with py3#246

Merged
Didou09 merged 10 commits intodevelfrom
bf-benchmark-w-py3
Nov 15, 2019
Merged

Benchmarks now with py3#246
Didou09 merged 10 commits intodevelfrom
bf-benchmark-w-py3

Conversation

@lasofivec
Copy link
Copy Markdown
Collaborator

Changes in benchmarks scripts:

  • instead of print(..., file=...) now using the more elegant write(...
  • took out the __init__.py file so that the directory is not packages (only useful for devs)
  • made default test smaller

@pep8speaks
Copy link
Copy Markdown

pep8speaks commented Nov 5, 2019

Hello @lasofivec! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-11-05 17:52:25 UTC

@codecov-io
Copy link
Copy Markdown

codecov-io commented Nov 5, 2019

Codecov Report

Merging #246 into devel will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##            devel     #246   +/-   ##
=======================================
  Coverage   43.93%   43.93%           
=======================================
  Files          70       70           
  Lines       21288    21288           
=======================================
  Hits         9352     9352           
  Misses      11936    11936

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 488b9f3...b4fd4fb. Read the comment docs.

@lasofivec
Copy link
Copy Markdown
Collaborator Author

Sorry for the horrible commit messages 🙈

@lasofivec lasofivec requested a review from Didou09 November 5, 2019 17:53
@lasofivec lasofivec self-assigned this Nov 5, 2019
Copy link
Copy Markdown
Member

@Didou09 Didou09 left a comment

Choose a reason for hiding this comment

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

Ok for me, seems cleaner
Are you sure with the f.write(...) ? I agree it is more elegant and pythonic but it seems quite less compact.
I'm ok with both, it's just because you raised the point and we might try to do the same in the future based on this discussion.

Did you try to launch the benchmark again after these changes ?
Since they are not in the unit tests, travis does not check them (unless my memory is failing)

@Didou09
Copy link
Copy Markdown
Member

Didou09 commented Nov 5, 2019

  • took out the __init__.py file so that the directory is not packages (only useful for devs)
  • made default test smaller

2 good ideas :-)

@lasofivec
Copy link
Copy Markdown
Collaborator Author

Ok for me, seems cleaner
Are you sure with the f.write(...) ? I agree it is more elegant and pythonic but it seems quite less compact.
I'm ok with both, it's just because you raised the point and we might try to do the same in the future based on this discussion.

Did you try to launch the benchmark again after these changes ?
Since they are not in the unit tests, travis does not check them (unless my memory is failing)

Yes I tried them, to be on the safe side, could you try them as well ?

@Didou09
Copy link
Copy Markdown
Member

Didou09 commented Nov 15, 2019

Ok, I coul try them on the IRFM clusters, it works too 👍

@Didou09 Didou09 merged commit dfd4825 into devel Nov 15, 2019
@Didou09 Didou09 deleted the bf-benchmark-w-py3 branch November 15, 2019 09:14
@Didou09 Didou09 mentioned this pull request Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants