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

Refactor runner #159

Merged
merged 3 commits into from
Nov 8, 2022
Merged

Refactor runner #159

merged 3 commits into from
Nov 8, 2022

Conversation

andreab1997
Copy link
Contributor

We want to massively simplify runnner.py:

  • In get_output we want to avoid the rotations: if needed they should be done afterwards.
  • In __init__ we just need to skip the last part in which targetpids, inputpids, targetgrid and inputgrid are collected.
    @felixhekhorn @alecandido

@codecov-commenter
Copy link

codecov-commenter commented Nov 4, 2022

Codecov Report

Merging #159 (bd846e2) into develop (e3466fa) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #159      +/-   ##
===========================================
- Coverage    99.95%   99.95%   -0.01%     
===========================================
  Files           97       97              
  Lines         4519     4496      -23     
===========================================
- Hits          4517     4494      -23     
  Misses           2        2              
Flag Coverage Δ
isobench 54.53% <100.00%> (-0.10%) ⬇️
unittests 99.95% <100.00%> (-0.01%) ⬇️

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

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

@alecandido
Copy link
Member

In get_output we want to avoid the rotations: if needed they should be done afterwards.

In particular, in get_output only the basic three lines of computation and return will survive, keeping it minimal.

In any case, this runner will be completely reworked in the context of #138

@felixhekhorn felixhekhorn added enhancement New feature or request refactor Refactor code output Output format and management labels Nov 4, 2022
@felixhekhorn felixhekhorn changed the base branch from master to develop November 4, 2022 13:56
@andreab1997
Copy link
Contributor Author

andreab1997 commented Nov 7, 2022

This should be ok now, we just need to fix the tests. Before doing that, can you confirm that this is what we want? @felixhekhorn @alecandido

@alecandido
Copy link
Member

This should be ok now, we just need to fix the tests. Before doing that, can you confirm that this is what we want? @felixhekhorn @alecandido

Yep, I confirm :)

@andreab1997
Copy link
Contributor Author

I am merging, thanks for this :)

@andreab1997 andreab1997 merged commit 66fcebc into develop Nov 8, 2022
@felixhekhorn felixhekhorn deleted the refactor_runner branch January 5, 2023 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request output Output format and management refactor Refactor code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants