Add method to retrieve the significant directory count

[melissalinkert]

See ticket #10296.

[joshmoore]

/cc @mtbc

[ghost]

Split directly on File.separatorChar?

[melissalinkert]

The above is incorrect (should be "\\\\" instead of "\\"), but splitting on File.separator directly is not quite right because of:

http://findbugs.sourceforge.net/bugDescriptions.html#RE_CANT_USE_FILE_SEPARATOR_AS_REGULAR_EXPRESSION

Fixing the backslash escaping in just a second.

[ghost]

Other than the minor comments above, it looks fine. I'm unsure how best to test this though--do we need a server-side usage of this first so we can test it with FS?

[mtbc]

Hopefully today I could use it in my code and make a PR that uses it testably in the server and includes the commits from this PR.

[mtbc]

Can I think of this as a static method -- does reader state matter at all?

[joshmoore]

A pre-setId method, so nearly static.

[melissalinkert]

It can be called before or after setId; the current reader state makes no difference.

[mtbc]

do we need to start the comment with two asterisks so that Javadoc can pick it up?

(oh, no, sorry, just as a normal comment Javadoc will pick up the doc from the interface, I think, I was just wrongfooted by the Javadoc-style "see")

[mtbc]

Looks good so far but I'll actually test it a bit today and comment here again later.

[mtbc]

So, if I take the MIAS data from squig's data_repo/One Plate/001-365700055641 then maxDirCount - dirCount will be 1, so the function will return 3, and we will keep all 3 directories of that common parent starting from data_repo. @melissalinkert, does that seem to match your understanding?

[mtbc]

Whoops, I now notice that @melissalinkert is marked as being away in the scheduling calendar. As far as I'm concerned, this PR is good to merge: if need be, the behavior can be tweaked at a later date. It would be bad if it retained too few directories, but in testing I've seen no problems of that kind and the code looks good.

[melissalinkert]

I don't think that's right - likely because getRequiredDirectories needs to correct for the Batchresults directory (directories?) being present. If my memory of that dataset structure is correct, the return value should be 1, not 3.

Up to you guys whether or not to merge, but either way that'll need to be fixed.
-----Original Message-----
From: Mark Carroll notifications@github.com
Date: Thu, 14 Feb 2013 04:35:58
To: openmicroscopy/bioformatsbioformats@noreply.github.com
Reply-To: openmicroscopy/bioformats reply@reply.github.com
Cc: Melissa Linkertmelissa.linkert@gmail.com
Subject: Re: [bioformats] Add method to retrieve the significant directory
count (#374)

So, if I take the MIAS data from squig's data_repo/One Plate/001-365700055641 then maxDirCount - dirCount will be 1, so the function will return 3, and we will keep all 3 directories of that common parent starting from data_repo. @melissalinkert, does that seem to match your understanding?

---

Reply to this email directly or view it on GitHub:
#374 (comment)

[mtbc]

Too small a return value would be a blocker, but returning 3 instead of 1, affecting MIAS only, makes me vote for merging so we get this function in and format-specific improvements can follow later.

[mtbc]

One way to test this is to also apply ome/openmicroscopy#751 and view the paths created server-side under ManagedRepository after uploads.

[melissalinkert]

I was mistaken; the return value of getRequiredDirectories for the above dataset should be 2 as One Plate/001-365700055641/ is what needs to be preserved. Should be fixed now.

[mtbc]

Still good to merge for me.

[joshmoore]

Thanks, guys. Merging and bumping the submodule pointer.