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 find.turn.point.R and find.tol.time.R and recover.weaker.R #91

Merged
merged 58 commits into from
Aug 17, 2022

Conversation

wverastegui
Copy link

@wverastegui wverastegui commented Aug 8, 2022

Refactored find.tol.time.R and find.turn.point.R. The pastecs library has been added to use the Turnpoint function in find.turn.point.R, that simplifies the code. Added pastecs to dependencies.

Closes #90
Closes #77

This PR now also includes the changes from #92 to fix the overarching test cases, as fixing the things for this PR would require changes in other functions, so I decided to append all changes from #92 here.

  • Extracted all functions performing substeps of the computation
  • Renamed variables to better reflect the data they actually contain
  • Disabled parallelism in test case for better debugging
  • Working toward isolating executions on individual files to ease implementing a parallel version in Galaxy
  • Added documentation to all extracted functions
  • Added stubs for further simplifications which might be more drastical code changes --> those need to be validated in extended tests

Closes #74

@hechth hechth added the refactoring Refactor existing code label Aug 10, 2022
@wverastegui
Copy link
Author

find.turn.pint.R was reverted to refactored version, Turnpoint function prevents passing the tests. Changelog file was updated accordingly (96c64ae)

conda/meta.yaml Outdated Show resolved Hide resolved
conda/environment-dev.yaml Outdated Show resolved Hide resolved
conda/meta.yaml Outdated Show resolved Hide resolved
Copy link

@xtrojak xtrojak left a comment

Choose a reason for hiding this comment

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

Consider spending more time with the merge, I think the final version could benefit from both your and my refactored versions.

DESCRIPTION Outdated Show resolved Hide resolved
@hechth
Copy link
Member

hechth commented Aug 12, 2022

Consider spending more time with the merge, I think the final version could benefit from both your and my refactored versions.

Thanks for the hint - I'll have a better look and see how to merge it better! Thanks for paying attention to details :)

@hechth hechth changed the title Refactor find.turn.point.R and find.tol.time.R Refactor find.turn.point.R and find.tol.time.R and recover.weaker.R Aug 16, 2022
@hechth hechth requested a review from xtrojak August 16, 2022 17:56
R/find.tol.time.R Outdated Show resolved Hide resolved
Co-authored-by: Matej Troják <trojak@mail.muni.cz>
@hechth hechth merged commit e55a447 into RECETOX:master Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Refactor existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor find.turnpoints Refactor find.tol.time refactor recover.weaker.R
3 participants