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

Refactoring #35

Open
1 of 2 tasks
DanielAndreasen opened this issue Nov 24, 2020 · 5 comments
Open
1 of 2 tasks

Refactoring #35

DanielAndreasen opened this issue Nov 24, 2020 · 5 comments

Comments

@DanielAndreasen
Copy link
Contributor

DanielAndreasen commented Nov 24, 2020

To code could use some refactoring. This is quite important, since it hopefully will encourage others to add new features.
Refactoring here includes:

  • Cleaning the code (removing unused code anf following PEP8)
  • Removing duplications of code, by creating functions/classes (preferrably in new files)
@dnil
Copy link
Contributor

dnil commented Nov 24, 2020

Thanks for looking - and I surmise for using the program? 👍🏻
I can't speak for @J35P312 but I have a hard time seeing things like that being anything but most welcome PRs! 😅

@DanielAndreasen
Copy link
Contributor Author

Yes, we use it. It's a quite cool program 😃
I added a feature some months ago (only allow variants during query with a frequency lower than a given threshold), and at the same time, I started doing a lot of refactoring. So now I have a local branch called cleanup. I hope we can have a look at this together at some point. Refactoring without testing is a bit risky, so we need good code review.

@dnil
Copy link
Contributor

dnil commented Nov 24, 2020

Sounds great! Right, agreed; for most other projects that got this far we have switched over if not to test driven so at least mandating that each new PR leaves the code more tested than before. That would be lovely here as well..

@DanielAndreasen
Copy link
Contributor Author

I agree. Maybe we should open a new issue regarding testing?
I would say it is going to be easier to start unit testing after refactoring

@J35P312
Copy link
Owner

J35P312 commented Nov 24, 2020

Hello!
Yes, I agree, refactoring is really needed, especially for collaboration!
Thanks a lot! Feel free to tell us when the cleanup branch is ready, or if you have questions, it sounds excellent!

I have a couple of tests I use to run, like querying a vcf with itself as a db, using strange chromosome names , etc, I can do so once you are ready.

I agree, the code is too messy for unit testing right now.

Thanks again, it will be exciting to hear more about the refactoring.

DanielAndreasen pushed a commit to DanielAndreasen/SVDB that referenced this issue Nov 24, 2020
@DanielAndreasen DanielAndreasen mentioned this issue Nov 24, 2020
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

3 participants