Place Alternative Names handling

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Place Alternative Names handling

prculley
Gentlemen;
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.

The code in question is in place.py in a two spots.
    def _merge_alt_names(self, acquisition):
        """
        Add the main and alternative names of acquisition to the alternative
        names list.

        :param acquisition: instance to merge
        :type acquisition: :class:'~.place.Place
        """
        if acquisition.name and \                     #prc comment 1 below
           (acquisition.name not in self.alt_names):  #prc comment 2 below
            self.alt_names.append(acquisition.name)

        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;
  1. 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.

  2. 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.

Paul Culley

------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Place Alternative Names handling

enno
Paul,
> Gentlemen;
> 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?

cheers,

Enno


------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel