Generalize position finder#5
Conversation
petebankhead
left a comment
petebankhead
left a comment
There was a problem hiding this comment.
This looks good and seems to work well.
I've added some comments for how it might be more general. If you think some are worthwhile but would take too long to implement now, they could be moved to become GitHub issues or potential implementation later.
Two other things:
- The ReadMe should point to the example scripts, and explain specifically how to handle Vectra 2 and Vectra 3 images (since those are the primary intended use for now)
- I find the dialog a bit confusing, in that I choose 'OK' before I have selected images - so it isn't clear what I'd be stitching.
To address 2 you could add a file chooser prompt to the dialog, although I think an easier option would be to change the button text from OK to Choose files.
Alternatively, what happens after pressing OK could be explained with a text prompt on the dialog.
| This extension adds support for combining TIFF images based on their "XResolution", "XPosition", "YResolution", "YPosition", "ImageWidth", and "ImageLength" tags. | ||
| This extension adds support for combining TIFF images based on their names and "XResolution", "XPosition", "YResolution", "YPosition", "ImageWidth", and "ImageLength" tags. | ||
|
|
||
| The extension is intended for QuPath v0.6 and later. |
There was a problem hiding this comment.
This PR means it is compatible only with v0.7.0.
Is it worth delaying the breaking change, so that we can have a v0.6.0-compatible release?
There was a problem hiding this comment.
This PR is actually v0.6.0 compatible, but two previous PR weren't: the TIFF writer PR, and the use of the new GeneralTools.moveToTrash()PR.
| public class PathPositionFinder implements PositionFinder { | ||
|
|
||
| private static final Logger logger = LoggerFactory.getLogger(PathPositionFinder.class); | ||
| private static final Pattern POSITION_IN_NAME_PATTERN = Pattern.compile("\\[([\\d.]+),([\\d.]+)]"); |
There was a problem hiding this comment.
This looks like it is specific to Vectra file names. Should the class name be changed to reflect this, e.g. VectraFilenamePositionFinder?
Alternatively, this could aim to be more general by iterating through alternative patterns, e.g. to include x=123. That isn't necessary since you allow the user to provide their custom position finder, but if the name is general then I think the behavior should try to match that generality.
[See https://imagej.net/plugins/image-stitching for some examples of different potential layouts... although here x and y are for the grid position, and actually this would need to be multiplied by the image width or height (I presume).]
There was a problem hiding this comment.
I made PathPositionFinder more generic (it is now FilenamePatternPositionFinder) while still having a predefined Vectra pattern. Are there other patterns that should be predefined?
|
The comments were addressed |
2 participants
Add support for more images that contain the tile position in the image name (like
...[x,y]....tiffwherexandyare positions in micrometers).Add a new UI parameter to define how to determine the tile position (TIFF tags like before or image name as mentionned above):
In scripting, it is now possible to completely define how to determine the tile position.