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.

Sign up for GitHub

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

Jump to bottom

Update get_input() function to get_data_path() and implement it within modules #63

Merged
merged 17 commits into from
May 25, 2023
Merged

Update get_input() function to get_data_path() and implement it within modules #63

merged 17 commits into from
May 25, 2023

Conversation

axiomcura
Copy link
Member

@axiomcura axiomcura commented May 23, 2023 •
edited
Loading

About

This PR introduces an update to the previously developed helper function get_inputs() #60 . A clever implementation was suggested by @d33bs that resulted common.smk module have less code written!

Because of this change, lots of other components needed to be changed as well.

There are the changes:

  • renamed get_input to get_data_path()
  • changed data_type parameter to input_type
  • removed commented documentation and placed it under the get_data_path() function
  • replaced inputs and ouputs parameter
  • removed output constants in common.smk module
  • workflow rule all constants are now replaced with get_data_path() function calls

associated issues

@axiomcura axiomcura added the enhancement New feature or request label May 23, 2023
@axiomcura axiomcura marked this pull request as ready for review May 24, 2023 12:43
@axiomcura axiomcura requested a review from d33bs May 24, 2023 15:51
d33bs
d33bs approved these changes May 24, 2023
View reviewed changes
Copy link
Member

@d33bs d33bs left a comment

Choose a reason for hiding this comment

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

Nice work! I left a few comments and suggestions with this review, overall it LGTM! Please don't hesitate to let me know if you have any questions.

workflows/scripts/annotate.py Outdated Show resolved Hide resolved
workflows/rules/common.smk Outdated Show resolved Hide resolved
workflows/rules/common.smk Outdated Show resolved Hide resolved
workflows/rules/common.smk Outdated Show resolved Hide resolved
workflows/rules/common.smk Show resolved Hide resolved
workflows/rules/common.smk Outdated Show resolved Hide resolved
workflows/rules/common.smk Outdated Show resolved Hide resolved
workflows/rules/common.smk Outdated Show resolved Hide resolved
workflows/rules/common.smk Show resolved Hide resolved
axiomcura and others added 9 commits May 24, 2023 13:16
@axiomcura @d33bs
Update workflows/rules/common.smk
593623b 
Co-authored-by: Dave Bunten <ekgto445@gmail.com>
@axiomcura @d33bs
Update workflows/rules/common.smk
1ecf7a7 
Co-authored-by: Dave Bunten <ekgto445@gmail.com>
@axiomcura @d33bs
Update workflows/rules/common.smk
dd0c0cc 
Co-authored-by: Dave Bunten <ekgto445@gmail.com>
@axiomcura @d33bs
Update workflows/rules/common.smk
b53fe74 
Co-authored-by: Dave Bunten <ekgto445@gmail.com>
@axiomcura
applied changes and suggestions
2f395c2
@axiomcura
applied changes
4f07672
@axiomcura
fix "path" bug
b425c63
@axiomcura
update commonk.smk
0acb98e
@axiomcura
applied comments and suggestions
d9c0d99
@axiomcura
Copy link
Member Author

@d33bs I have applied all comments and suggestions, merging! Thanks!

@axiomcura axiomcura merged commit cbfa350 into WayScience:main May 25, 2023
@axiomcura axiomcura deleted the get_input_update branch May 25, 2023 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@d33bs d33bs d33bs approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@axiomcura @d33bs