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

Clean up package structure and optimize script #5

Open
jeff-mandell opened this issue Feb 5, 2024 · 6 comments
Open

Clean up package structure and optimize script #5

jeff-mandell opened this issue Feb 5, 2024 · 6 comments

Comments

@jeff-mandell
Copy link

Hi,
Thanks for making this package. I develop integrative methods to quantify cancer evolution (see https://townsend-lab-yale.github.io/cancereffectsizeR/index.html), and your armSCNA peak-finding algorithm and fitness measure will likely be useful for my work.

In going through your code, I saw that the GEV calculations could be optimized to make the algorithm run a lot faster. After making the change, I'm able to run the pan-cancer analysis in a few minutes on a single core, compared to several hours on multiple cores. The codebase could also be improved by restructuring the file organization to match a standard R package (which would enable simpler installation), and it would also be nice if a lot of hard-coded options and file directories were changed so that default outputs don't go into the package directory itself.

If I make a version with these changes, would you be willing to review a pull request?

Thanks,
Jeff

@shahab-sarmashghi
Copy link
Contributor

Hi Jeff,

I'd love to see such improvements in running time and codebase/UI, and I'm sure the whole community will benefit from that. That being said, let me first discuss this internally before you put more efforts into this. This might take a week or two but I will surely get back to you.

Best,
Shahab

@shahab-sarmashghi
Copy link
Contributor

@jeff-mandell we welcome your contribution. I'll be more than happy to review your PR and accept it after some testing to make sure the algorithm works as expected.

@jeff-mandell
Copy link
Author

Great, I will try to have something over by next week.

@jeff-mandell
Copy link
Author

I created a pull request; see #6.

@jeff-mandell
Copy link
Author

Hi @shahab-sarmashghi, is there anything I could do that would be helpful in facilitating review of the pull request? There's no rush on my end, although eventually it would be nice to point people to the original package rather than my fork

@shahab-sarmashghi
Copy link
Contributor

Hi @jeff-mandell, sorry I've just been very busy. Will try to get through these in the next week or two.

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

No branches or pull requests

2 participants