for addendum in acquisition.alt_names: if addendum not in self.alt_names: #prc comment 2 below self.alt_names.append(addendum)
def add_alternative_name(self, name): """ Add a name to the alternative names list.
:param name: name to add :type name: string """ if name not in self.alt_names: #prc comment 2 below self.alt_names.append(name)
I have the following observation;
this is always true since Place.__init__ puts PlaceName object here. I think this should be if acquisition.name.value # this would check for an actual assignment, not just an empty entry.
when testing for 'not in' python says that is equivalent to checking for 'not' ('is' or '==') for each item in the list. So it should have caught the duplicate names. I delved deeper and discover that we never defined a '__eq__' operator overload for the PlaceName class (or the SecondaryObject etc.) so the '==' ends up as 'is'. Shouldn't you be comparing the actual contents to each other? I did a quick test by defining '__eq__' to be the same as 'is_equal' in the PlaceName class and I stopped getting the duplicate Place alt_name entries.
If you think I am on the right track, I can submit a bug report and a code patch.
> I have been investigating bug 9173 and came across two puzzling issues
> in some of my tests.
> 1) I'm seeing totally blank Place alt_name entries
> 2) I also see duplicate alt_name entries in the list.
> I think that the code is supposed to avoid this, but I may
> misunderstanding the intention.
I think you're right. There's no sense in creating empty name entries,
nor in duplicates.
I also read Sam's comment on #9173, saying that it's best to create a
separate report for this.
While on the subject, there's another issue that is bugging me a bit.
That one occurs when there is an alternative place in a 3.4 type
database. When that database is converted to or imported into Gramps
4.2, the alternative place is still in 3.4. format.
I would really like to remove this inconsistency in 5.1. Would you agree?