View Issue Details

IDProjectCategoryView StatusLast Update
0002687Slicer4Core: MRMLpublic2017-09-27 10:34
ReporterpinterAssigned Toalexy 
PrioritynormalSeverityminorReproducibilityalways
Status feedbackResolutionopen 
Product VersionSlicer 4.1.1 
Target VersionbacklogFixed in Version 
Summary0002687: Saving a scene that contains nodes with the same name results in data loss
Description

When saving a scene that contains two nodes that have the same name, then Slicer prompts for overwriting the already saved file, because it wants to use only the node name as filename.
When loading the saved scene, one of those nodes will contain invalid data.

For example: I have two models called Bladder. I save the scene, and I choose to overwrite. Then after loading the scene, the first 'Bladder' model will contain the same data as the second one (because they were loaded form the same file). If I choose not to overwrite then the first one will appear in both nodes.

Although the file name can be changed in the 'Save Scene and Unsaved Data' form, it would be better to have different names in the first place.

Additional Information

A question: Is there a use case where having the same name for more MRML nodes is beneficial? It not only causes the error described above, but also leads to confusion. Is it a good idea not to allow this to happen (force the second node to have a different name, for example to force a unique name when adding nodes to the scene)?

TagsNo tags attached.

Relationships

related to 0002816 feedbackalexy Restoring scene views from MRB 

Activities

jcfr

jcfr

2012-10-25 09:25

administrator   ~0006746

Reminder sent to: kikinis

Hi Ron,

Regarding the question of Csaba, what do you think ?

It seems having identical name leads to confusion, isn't it ?

Thanks

kikinis

kikinis

2012-10-25 10:10

developer   ~0006749

The original scenes were numbered sequentially. Sceneview1, Sceneview2, etc. This feature was lost somewhere. As far as I am concerned, it should be restores. The name of the sceneview is used as the name of the thumbnail and use of the same name can lead to overwriting a thumbnail.

alexy

alexy

2012-12-18 05:41

developer   ~0007519

Csaba, how do you create nodes with the same name? If you just data from a file Slicer should add _1 to the second node with the same name. Do you create the nodes programmatically?

pinter

pinter

2012-12-18 06:39

developer   ~0007524

Yes, I create volume, model and color nodes in the DicomRtImport module, when loading data from the DICOM database to Slicer. When I load the same series twice, then their names are the same.

I could call vtkMRMLScene::GenerateUniqueName, but without explicitly calling it, the names remain the same. If there is really no use case when objects with same names are required (in my case it is not required, but I cannot see the need of every user), then the GenerateUniqueName function should be called when setting the name of a node.

alexy

alexy

2012-12-18 09:19

developer   ~0007533

I cannot think why would vtkMRMLNode::SetName() not do that, but lets discuss it at slicer hangout.

pieper

pieper

2012-12-18 13:09

administrator   ~0007551

As discussed on the hangout, we unique names in the user interface shouldn't be enforced, but unique file names should be.

Alex will be looking at centralizing the handling of unique filenames as part of issue 0002816.

alexy

alexy

2013-08-27 10:54

developer   ~0009619

Last edited: 2013-08-27 10:55

After discussing this issue with Steve there are somewhat conflicting requirements of keeping file names in sync with Storable node names and keeping file names unique. There are different use cases, and the logic of satisfying both requirements becomes complicated.
Csaba, it seems that what you really want if for DICOM import to create unique names. Would that satisfy your use case?

pinter

pinter

2013-08-27 11:11

developer   ~0009623

There are many possible workarounds for this issue, there is no question about that.
The point here is that

  1. MRML nodes having the same name is allowed
  2. Files having the same names are not allowed
    These two are in conflict, thus there is a bug when saving a scene where the same name appears in more than one MRML node.
    I think the correct solution would be to ensure unique names for MRML nodes in general. Is there an advantage in allowing duplicate MRML node names?
alexy

alexy

2013-09-17 05:16

developer   ~0009964

It seems like we need to discuss enforcing unique MRML names.

pieper

pieper

2013-09-17 08:12

administrator   ~0009969

Why not just add a validator step to the save dialog? It seems that if we found that this issue existed we could put up a simple table where every filename is made unique. Then let the user edit them if they don't like them. This should include scene views.

Another thing we might consider: if the nodes with the same names are in different parts of the subject hierarchy they could be created in corresponding subdirectories on disk (or given names that concatenate the hierarchy name with the node name). We could also prefix with the sceneview name.

pinter

pinter

2013-09-17 08:41

developer   ~0009970

I like the idea of different subdirs basd on major hierarchy branches, but it's not something we can implement now.

Is there an advantage in allowing duplicate MRML node names in the scene?

pieper

pieper

2013-09-17 08:45

administrator   ~0009971

The only advantage of duplicate names would be exactly in the hierarchy case (I thinnk preop/kidney and postop/kidney is better than preop/kidney and postop/kidney_1). Also we've never ruled it out before so some code could break if we start insisting on uniqueness.

pinter

pinter

2013-09-17 09:01

developer   ~0009972

OK, the subdirectories and the reconciliation table (in case there is a duplication within a directory) sound good.
Thanks!

lassoan

lassoan

2013-09-17 09:50

developer   ~0009973

I think for now when there is a filename conflict we should just show an error popup to inform the user and reject the saving (and let the user resolve the conflict manually and retry the saving).

Later, if we have time, we could implement a "Resolve" button in the error popup, which would call an algorithm that would generate unique filenames by adding a prefix based on names of parent hierarchy nodes and/or adding some generic postfixes (_1, _2, ...).

pieper

pieper

2013-09-17 13:38

administrator   ~0009991

I agree - the biggest issue now is that the save is half-finished before the user is notified. They can still re-save and fix the paths, but it's awkward.

alexy

alexy

2014-06-13 09:48

developer   ~0012055

Since it is late in the release and the logic for enforcing unique names is not crystal clear, moving to 4.5

jcfr

jcfr

2016-10-12 15:10

administrator   ~0014193

Last edited: 2016-10-12 15:11

View 2 revisions

I confirm this is still an issue with r25435

Step to reproduce:

  • Load MRHead
  • Apply Gaussian blue creating a new volume as output
  • Rename the output to have the same name
  • Save
  • Reload
  • Both MRHead are identical ... whereas we could expect to have both loaded

Issue History

Date Modified Username Field Change
2012-10-25 07:13 pinter New Issue
2012-10-25 07:13 pinter Status new => assigned
2012-10-25 07:13 pinter Assigned To => alexy
2012-10-25 09:25 jcfr Note Added: 0006746
2012-10-25 10:10 kikinis Note Added: 0006749
2012-12-08 10:08 jcfr Target Version => Slicer 4.2.3
2012-12-18 05:41 alexy Note Added: 0007519
2012-12-18 05:41 alexy Status assigned => feedback
2012-12-18 06:39 pinter Note Added: 0007524
2012-12-18 09:19 alexy Note Added: 0007533
2012-12-18 13:09 pieper Note Added: 0007551
2012-12-18 13:09 pieper Relationship added related to 0002816
2013-02-12 09:37 jcfr Target Version Slicer 4.2.3 => Slicer 4.3.0
2013-08-27 10:54 alexy Note Added: 0009619
2013-08-27 10:55 alexy Note Edited: 0009619
2013-08-27 11:11 pinter Note Added: 0009623
2013-08-30 12:14 jcfr Target Version Slicer 4.3.0 => Slicer 4.3.1
2013-09-17 05:16 alexy Note Added: 0009964
2013-09-17 05:16 alexy Target Version Slicer 4.3.1 => Slicer 4.4.0
2013-09-17 08:12 pieper Note Added: 0009969
2013-09-17 08:41 pinter Note Added: 0009970
2013-09-17 08:45 pieper Note Added: 0009971
2013-09-17 09:01 pinter Note Added: 0009972
2013-09-17 09:50 lassoan Note Added: 0009973
2013-09-17 13:38 pieper Note Added: 0009991
2014-06-13 09:48 alexy Note Added: 0012055
2014-06-13 09:48 alexy Target Version Slicer 4.4.0 => Slicer 4.5.0-1
2015-11-02 11:27 jcfr Target Version Slicer 4.5.0-1 => Slicer 4.6.0
2016-10-12 15:10 jcfr Note Added: 0014193
2016-10-12 15:11 jcfr Note Edited: 0014193 View Revisions
2016-10-12 15:11 jcfr Target Version Slicer 4.6.0 => Slicer 4.7.0
2017-09-27 10:34 lassoan Target Version Slicer 4.7.0 => backlog