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

Implement new output format #77

Merged
merged 9 commits into from
Oct 25, 2021
Merged

Implement new output format #77

merged 9 commits into from
Oct 25, 2021

Conversation

alecandido
Copy link
Member

@alecandido alecandido commented Oct 19, 2021

Closes #76

  • add tests for coverage d620a68
  • dump eko version into metadata (add to get_raw()) 7d48679

@alecandido alecandido changed the base branch from master to develop October 19, 2021 16:09
@codecov
Copy link

codecov bot commented Oct 19, 2021

Codecov Report

Merging #77 (d620a68) into develop (97e3149) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #77      +/-   ##
===========================================
+ Coverage    99.25%   99.27%   +0.01%     
===========================================
  Files           29       29              
  Lines         2149     2200      +51     
===========================================
+ Hits          2133     2184      +51     
  Misses          16       16              
Flag Coverage Δ
unittests 99.27% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/eko/output.py 100.00% <100.00%> (ø)

Copy link
Contributor

@felixhekhorn felixhekhorn left a comment

Choose a reason for hiding this comment

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

add tests for coverage
dump eko version into metadata (add to get_raw())

this definitely needs to be addressed before merging (esp. the first ;-) )

src/eko/output.py Outdated Show resolved Hide resolved
src/eko/output.py Outdated Show resolved Hide resolved
src/eko/output.py Show resolved Hide resolved
@alecandido
Copy link
Member Author

this definitely needs to be addressed before merging (esp. the first ;-) )

For sure, but I wanted to get your thoughts on current implementation actually :)

@felixhekhorn
Copy link
Contributor

For sure, but I wanted to get your thoughts on current implementation actually :)

Looks reasonable!

@alecandido
Copy link
Member Author

The last pylint complaint can be solved by moving to poetry, but for sure it will be done in another PR

@alecandido
Copy link
Member Author

@felixhekhorn I provided the test for the new output (a bit stupid but working) d620a68.

Nevertheless, looks like there is not any longer a 100% coverage, because of operator_matrix_element. I only touched output.py, test_output.py and sandbox.py, so the gap should have been preexisting.
Maybe you want to fill.

In any case, here I'm ready to merge.

@alecandido alecandido merged commit e324305 into develop Oct 25, 2021
@delete-merged-branch delete-merged-branch bot deleted the feature/output-revamp branch October 25, 2021 13:36
@alecandido alecandido mentioned this pull request Mar 18, 2022
4 tasks
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.

Output format revamped
2 participants