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

Result passing property added for other projects. #316

Closed
wants to merge 1 commit into from

Conversation

crcrewso
Copy link
Contributor

@crcrewso crcrewso commented Oct 7, 2020

The intent of this patch is to create a easy to maintain self building dictionary, that can be called by external classes or frameworks.

Basic format would be:

analyzed_cbct[<module>][<result type or sub module>]

I.e.

analyzed_cbct['CTP404']['HUs']['Air'] 

would store the analyzed measure for the air volume of the CTP404 module.

At all points the indices would be human readable, and for modules with more or less roi's then the function could be redefined from the inherited module.

Documentation of the human readable names would have to be added to the readthedocs of course.

@coveralls
Copy link

coveralls commented Oct 7, 2020

Coverage Status

Coverage decreased (-0.5%) to 83.321% when pulling 5308c4a on crcrewso:Develop into 14a5296 on jrkerns:master.

if self._has_module(CTP515):
result["CTP515"] = self.ctp515.results_dict
if self._has_module(CTP528CP504):
result["CTP528"] = self.ctp528.result_dict
Copy link
Owner

Choose a reason for hiding this comment

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

typo here

@jrkerns
Copy link
Owner

jrkerns commented Nov 20, 2020

Corrected:
Thanks for the pull request. If I'm reading this right, you'll need to actually call myct.results_dict['CTP404']..., correct?

If indeed this is what you want, you'll need to add unit tests (probably here) or per-phantom (probably here) and update the docs (I'd put it in the "view the results" section here.

@crcrewso
Copy link
Contributor Author

Corrected:
Thanks for the pull request. If I'm reading this right, you'll need to actually call myct.results_dict['CTP404']..., correct?

If indeed this is what you want, you'll need to add unit tests (probably here) or per-phantom (probably here) and update the docs (I'd put it in the "view the results" section here.

Yes, I do intend to support all phantoms in the same way, creating a unified consistent jsonable object for any cbct analysis. Will amend this pull request as I have time.

@jrkerns
Copy link
Owner

jrkerns commented Feb 9, 2021

Good news on this front. I'm working on adding integration via APIs, so a dict of results (converted to JSON) can be helpful here. I'll tackle this once I can unify the workflow.

@jrkerns
Copy link
Owner

jrkerns commented Apr 6, 2021

Closed in #360

@jrkerns jrkerns closed this Apr 6, 2021
@crcrewso crcrewso deleted the Develop branch April 19, 2021 18:03
jrkerns added a commit that referenced this pull request Dec 11, 2023
RAM-3108 Fix order of text in PDF to match images

Approved-by: Randy Taylor
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

Successfully merging this pull request may close these issues.

3 participants