OxfordIonTrapGroup / ndscan

N-dimensional scans for ARTIQ
GNU Lesser General Public License v3.0
19 stars 16 forks source link

utils.shorten_to_unambiguous_suffixes: Fix bug by simplified rewrite, document #384

Closed dnadlinger closed 7 months ago

dnadlinger commented 7 months ago

The added ["bar", "foo/bar"] test case failed depending on the order in the old implementation.

This new, much more straightforward implementation might lead to subtly different disambiguations in some cases, but if such cases in fact do exist, this seems like a worthwhile trade-off to make, since we never actually document the details of the shortening algorithm in the user visible parts, or state that it is unchanging. If the changes do turn out to be obnoxious for some established analysis pipeline, we could salvage the old implementation quite easily by skipping the "promotion" check if old == current, but rewriting this was quicker than convincing myself that this fix actually would address all the corner cases.