-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #651. Add title manager to ensure unique displayed titles. #1221
base: master
Are you sure you want to change the base?
Conversation
debf282
to
d0cf416
Compare
d0cf416
to
f701421
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have concerns about this PR.
What is the problem it is trying to solve?
Having unique names - but why do users want unique names, and does this solve their actual need?
During my testing, I got 2 identical suffixes #96
with 4 snapshots. While I accept this will be rare - how often do we want to accept collisions happening?
Will the random nature of the suffixes confuse users? For example, the first few I got all appeared to be just numbers #58
, #96
, #84
. Are users going to wonder what is going on with those numbers?
I believe the implementation done for the Acquisitions is a much cleaner user experience, and I think something similar would work better here.
This may not be needed in light of the acquisition index change and it may indeed be confusing to users. We can discuss that before merging (or not merging). However, I'm concerned that even if we decided it was a useful feature, it shouldn't ever have duplicate suffixes. So I'm wondering how that occurred. I tested by changing the minimum length to 1 and then creating multiple snapshots and when it got to a duplicated suffix out of the 0-9A-F set, it increased the length to 2. Did this happen when the '96' appeared for you, e.g. increase to length 3 to distinguish between the two '96's? |
Ok that makes sense - but it does highlight a problem with this scheme. I'm going to change this to draft while I think it through. |
How I tested:
Use Processing > Generate to create 2+ data items with same name. Take FFT of results to generate new data items. Run acquisition and take multiple snapshots and see that they have unique titles.