search

cutright / DVH-Analytics

A DICOM Database Application for Radiation Oncology
Other
82 stars 30 forks source link

Break out calculation from UI? #89

Closed darcymason closed 4 years ago

darcymason commented 4 years ago

Hi @cutright,

Thanks for your great work on this package. I'm working towards using the dose sum parts in another project, but have some conflicts (bokeh version) on installing.

I could probably work around it, but I was wondering if there was ever consideration of breaking out some of the calculation engine to a "-core" package or similar, to separate from the app/display interface.

No problem if not, I understand it would be added work to maintain separately. I could also just copy the relevant sections I need (with appropriate attribution and licence of course), but it is nice to keep up with updates automatically through a pip dependency.

If you have any interest in the idea, I would help with breaking out the dose summing at least, and other parts if I could gain more familiarity.

cutright commented 4 years ago

Hi @darcymason , many more thanks to you for pydicom!

I think what makes the most sense here is I'll work with @bastula to incorporate the dose summation into dicompyler-core. I think that would be an equivalent solution for you?

darcymason commented 4 years ago

Sure, that sounds great, but I'm happy to work with whatever is best for you.

cutright commented 4 years ago

Yeah, I think adding it to dicompyler-core would be way faster than breaking up DVHA at this point. Plus @bastula is on board with it. If I have any updates for the dicom dose sum, I'll just go through dicompyler-core.

cutright commented 4 years ago

@darcymason I've created a pull-request: https://github.com/dicompyler/dicompyler-core/pull/164

Please feel free to contribute or provide feedback if it's missing something. I'll work on documentation and test cases over the next week or so.

darcymason commented 4 years ago

Thanks, @cutright,

I had a quick look and it looks good - the DoseGrid class is what I needed. I have a couple of tests in the pipeline also that I may be able to contribute after that is merged in.

Thanks again.