search

mathesong / kinfitr

kinfitr: PET Kinetic Modelling Using R
Other
33 stars 7 forks source link

Adding new parameters: R1 and k2 for MRTM1 #21

Closed nakulrrraval closed 2 years ago

nakulrrraval commented 2 years ago

I needed the R1 and k2 for the MRTM1 for QC, hence proposing adding these parameters to the model.

mathesong commented 2 years ago

Hey! Awesome - thanks so much for the PR! I'd actually completely forgotten that these parameters could be extracted from the model, so this is really helpful to be able to provide them!

I'll accept the PR a little later this week when I have time, since I want to just additionally modify the functions (and the documentation) so that these parameters are only included if tstarIncludedFrames is left out or equal to the number of frames, since the R1 and k2 are only applicable for 1TC kinetics, in which case there should be no t*.

Thanks again! So great to get input like this! :D

nakulrrraval commented 2 years ago

Right! Did not think about that. Makes complete sense. Thank you so much for the wonderful tool. Cheers.

mathesong commented 2 years ago

The PR is now merged, and I've also updated it to make sure that it only adds those terms if we can assume 1TC dynamics. Thanks again for the addition!!

nakulrrraval commented 2 years ago

Nice! I have some more small ideas as well. Do you prefer PRs or would you like me to list them for you? Kinfitr is awesome! I have learned so much about KM because of it. Thanks again.

mathesong commented 2 years ago

Awesome - yeah, I'd love some feedback and ideas!

Regarding issues vs PRs, I think the usual way that people go about these things is to raise an issue first and propose the suggestion. And then, if the maintainer says it sounds like a good idea (or if there's already an open issue), then a PR can be submitted and linked to in the issue. This saves contributors the effort of working on something that the maintainer doesn't think belongs in the package, or doesn't want to maintain. I think with small things like this addition, it's fine to just submit a PR. But I do think that the conventional "etiquette" (not that I'm well versed in these things myself :upside_down_face: ) is first to submit an issue. Certainly that makes it easier for me to make suggestions prior to implementation.

So yeah, do raise some issues and make some suggestions - that would be awesome!

And thanks again for your PR and the kind words! :relaxed: