Nanostring-Biostats / SpatialOmicsOverlay

Tools for analyzing data on the image from NanoString GeoMx Digital Spatial Profiler (DSP).
MIT License
17 stars 4 forks source link

Allow string names for ROIs #54

Closed natmurad closed 11 months ago

natmurad commented 11 months ago

update readLabWorksheet to use read.csv because read.table does not read rois names column properly

includes index in bookendStr

remove as.numeric from annots and parseOverlayAttrs where it is not needed

maddygriz commented 11 months ago

Hi @natmurad,

Thanks for the updates. We have actually fixed both of these issues for the next Bioconductor release. These are fixed in the dev branch if you want to use/test this branch. There are other updates to this branch seen in the NEWS file. This branch has gone through our code review process but has not been through V&V yet.

Hope this helps and thanks again for your contribution. I'm closing this PR since we have already fixed the issues.

Maddy

natmurad commented 11 months ago

Not sure about which changes you did, but anyway, I just tested this version and it does not work. My corrections work.

Extracting XML Parsing XML - scan metadata Parsing XML - overlay data Generating Coordinates |++++++++++++++++++++++++++++++++++++++++++++++++++| 100% elapsed=03s
Warning messages: 1: In readLines(lw) : incomplete final line found on '/Users/LabWorksheet.txt' 2: In parseOverlayAttrs(omexml = xml, annots = annots, labworksheet = labWorksheet): Some AOIs do not match annotation file. Not Matched: DSP-1001660006334-A-A03, DSP-1001660006334-A-A05, DSP-1001660006334-A-A07, DSP-1001660006334-A-A09, DSP-1001660006334-A-A11, DSP-1001660006334-A-B01, DSP-1001660006334-A-B03, DSP-1001660006334-A-B06, DSP-1001660006334-A-B08, DSP-1001660006334-A-B10, DSP-1001660006334-A-B12, DSP-1001660006334-A-C02, DSP-1001660006334-A-C04

My changes do not work in this new version, tho.

maddygriz commented 11 months ago

It looks like it ran without erroring but that of course doesn't always mean that it completely worked. 13 ROIs were removed but how many did you start with?

natmurad commented 11 months ago

This is the output from this new dev version. It did not match the string ROIs that are those 13. I guess it is accepting or all names as strings, or all names as numeric because of some if you included, maybe? I do not know. I did not check this new code.

My corrections do not give this kind of warning because it recognizes any type of ROI names. It was the way that it worked with my dataset.

maddygriz commented 11 months ago

Do your ROI names potentially have either "\" or "=" characters in them? That is the only meaningful difference I see in our changes.

natmurad commented 11 months ago

Yes. 001#2 001#3a. It was not chosen by me.

maddygriz commented 11 months ago

Cool I see the issue on my end. I will have a new branch up with bug fix in the next day or so. Thanks for working with me to try and fix this.

maddygriz commented 11 months ago

I have a fix in, not reviewed, in the specialCharacterROIs branch.

natmurad commented 11 months ago

Thank you!