get num tissues #296
get num tissues #296
Conversation
histoqc/BasicModule.py
Outdated
|
|
||
|
|
||
| def countTissuePieces(s, params): | ||
| area_thresh = int(params.get("area_threshold", "1000")) |
There was a problem hiding this comment.
I think applying an area threshold here is redundant. Ideally small objects etc. will already be filtered out and the countTissuePieces method will just report what's there.
histoqc/BlurDetectionModule.py
Outdated
| # area_mean = areas.mean() | ||
| # else: | ||
| # nobj = area_max = area_mean = 0 | ||
| stat_rs = getMaskReginsStats(mask) |
There was a problem hiding this comment.
Looks like none of the stats are being added to the printList?
histoqc/BaseImage.py
Outdated
| @@ -341,3 +342,22 @@ def getDimensionsByOneDim(s: BaseImage, dim: int) -> Tuple[int, int]: | |||
| else: | |||
| w = int(dim * width / height) | |||
| return w, dim | |||
|
|
|||
|
|
|||
| def getMaskReginsStats(img, area_threshold: int = 0)-> dict: | |||
There was a problem hiding this comment.
regions misspelled
histoqc/BaseImage.py
Outdated
| @@ -206,6 +206,9 @@ def getImgThumb(self, size: str): | |||
| self[key] = getBestThumb(self, bx, by, target_dims, target_sampling_factor) | |||
| return self[key] | |||
|
|
|||
| ''' | |||
| the followings are helper functions | |||
There was a problem hiding this comment.
"following" not "followings"
|
Thanks Nan, Just a few small comments above. |
Hi @jacksonjacobs1, please review and confirm the changes. |
|
@choosehappy one quick question before I merge this in.... This PR adds a line to config v2.1 to count # of tissue pieces. I do not think we should modify config files while keeping the same filename. Conceptually, histoqc should always produce the same output if the user uses config v2.1. Do you agree? |
|
that is a very good, even philosophical, question at minimum I would say the same config file should produce the same numerical results, and in this case, simply counting the pieces of tissue isn't actually impacting the metrics on the other hand, the output file itself now changes - which is less than ideal ,but may be tolerable how do you want to address this? i think we need config files to be "fully featureD" so that folks can see how to use the different things. on the other hand, "2.1" was for the 2.1 release --- are we still on the 2.1 release if we keep adding things? at some point we should officially release (tag + code) e,g. 2.2, 2.3 etc - maybe having a config name based on the version is a bit chaotic and unsustainable? maybe it should all go into just config.ini and have the versioning "do its thing"? what do you think? |
|
I agree that the config file v2.1 naming convention is a bit chaotic. However, pushing changes into the config.ini kind of defeats the purpose of nan's unit tests: The unit tests were written specifically for a configuration file (in this case v2.1) which we assume does not change between commits. If we change config files in commits, we will also need to update the unit tests before each commit. |
I don't immediately know that this is a bad thing? if we're adding new features, we should be adding new unit tests to cover those features? so it somehow seems like this is the intended outcome? |
feature added: # of pieces of tissue #276
BaseImage.getMaskReginsStatsto get the based statistics for tissue areasBasicModule.countTissuePiecesto get # tissues in the current mask./histoqc/config/config_v2.1.inifiles to save#pieces_of_tissue