Skip to content

Modularize: Minor PR#214

Merged
grantmcdermott merged 5 commits intograntmcdermott:mainfrom
vincentarelbundock:modularize
Sep 10, 2024
Merged

Modularize: Minor PR#214
grantmcdermott merged 5 commits intograntmcdermott:mainfrom
vincentarelbundock:modularize

Conversation

@vincentarelbundock
Copy link
Copy Markdown
Collaborator

A minor PR with some more modularization/simplification:

  • Removed duplicated code
  • Removed unused arguments
  • type_*() data preparation function are now stored in a dictionary with switch-style selection.

Copy link
Copy Markdown
Owner

@grantmcdermott grantmcdermott left a comment

Choose a reason for hiding this comment

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

Very nice, thanks. One minor comment and then happy to merge.

Comment thread R/by_aesthetics.R
@vincentarelbundock
Copy link
Copy Markdown
Collaborator Author

thanks for the review. should be fixed now.

@grantmcdermott grantmcdermott merged commit 145ed58 into grantmcdermott:main Sep 10, 2024
@grantmcdermott
Copy link
Copy Markdown
Owner

Not so minor in the end... thanks again for the continued efficiency gains!

@vincentarelbundock
Copy link
Copy Markdown
Collaborator Author

no it really was minor. I just noticed that your suggestion could help us simplify something else too.

This was a simple PR, but I'll take the 35 lines of code removed :)

@vincentarelbundock vincentarelbundock deleted the modularize branch September 9, 2025 01:48
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.

2 participants