Bug Report

I have received a bug report! This is great news! Someone is paying attention!

Laurent wrote to me, saying that he reads my articles, and suggesting that a rename that includes a renaming from an old name to the same new name might not work. Here is my reply to him so far:

The following test passes:

def test_re_scope_per_laurent(self):
    s = XSet.from_tuples((('first', 'first_name'), ('last', 'last')))
    z = XSet.from_tuples((('ron', 'first'), ('jeffries', 'last'), ('serf', 'job')))
    r = z.re_scope(s)
    assert r.includes('jeffries', 'last')
    assert r.includes('ron', 'first_name')
    assert r.cardinality() == 2

So the re_scope does work. But, as you suggest, this rename test does not pass:

def test_rename_per_laurent(self):
    new_names = XSet.from_tuples((('first', 'first_name'), ('last', 'last')))
    person = XSet.from_tuples((('ron', 'first'), ('jeffries', 'last'), ('serf', 'job')))
    renamed = person.rename(new_names)
    assert renamed.includes('jeffries', 'last')
    assert renamed.includes('ron', 'first_name')
    assert renamed.includes('serf', 'job')

It loses the jeffries^last as you probably expected.

I’m starting an article now to address this bug. Thanks so much for paying such deep attention!

So, well done, Laurent. Let’s see if we can fix this test. I bet we can.

Let’s review how rename is written, so that we can see what it should have been:

    def rename(self, re_scoping_set: Self):
        # renames this set, not its contents sets
        old_names = self.scope_set()
        replaced_names = re_scoping_set.element_set()
        update = replaced_names | re_scoping_set
        renames = old_names ^ update
        return self.re_scope(renames)

I think that a simple fix will clearly work, removing any { old^old } from the re-scoping set, because this definition already puts them in. But perhaps there is a better way.

Suppose that

self = { x^a, y^b }

re_scoping_set = { a^aa, b^b }

Then

old_names = { a^a, b^b }

replaced_names = { a^a, b^b }

update = replaced_names | re_scoping_set
update = { a^a, b^b } | { a^aa, b^b }
update = { a^a, b^b, a^aa }

And then

renames = old_names ^ update
renames = { a^a, b^b } ^ { a^a, b^b, a^aa }
renames = { a^aa}

And that re_scoping set will drop the y^b entirely.

We cannot allow the re_scoping set to include any renames from old_name back to old_name, or they will cancel out when we do the ^ operation.

A simple fix is to remove any old -> old members from the re-scoping set. We’ll do this:

    def rename(self, re_scoping_set: Self):
        # renames this set, not its contents sets
        old_names = self.scope_set()
        re_scoping_set = re_scoping_set - old_names
        replaced_names = re_scoping_set.element_set()
        update = replaced_names | re_scoping_set
        renames = old_names ^ update
        return self.re_scope(renames)

Our failing test passes. We can commit: fix bug per Laurent causing erroneous renames when re-scoping set included a rename from old to old.

However, we are now up to five set operations, no, six, to do this job. It’s an impressive bit of set theory, but would this be better:

    def rename(self, re_scoping_set: Self):
        new_tuples = []
        for e, s in self:
            skipped = True
            for old, new in re_scoping_set:
                if old == s:
                    skipped = False
                    new_tuples.append((e,  new))
            if skipped:
                new_tuples.append((e, s))
        return XSet.from_tuples(new_tuples)

Well. On the one hand, we can kind of walk through that and understand it. The set-theory one is harder to think through. I don’t love it, though. Not going to commit it. I don’t like it well enough.

If I recall the article where I originally did rename, it was entitled “Hold my chai!”

That’s a pretty solid indication that I was being clever, and clever is what gets you a support call at 2 AM or an email from Laurent. Let’s see if we can devise a better version, using set theory.

Let’s consider what we want. The re_scoping set can include old -> new pairs, and we wish to allow old -> old.

The rename operation, by definition, should include all the old names that are not mentioned in the re-scoping set.

How about this version:

    def rename(self, old_to_new_re_scope_set: Self):
        # renames this set, not its contents sets
        original_names = self.scope_set()
        originals_renamed = old_to_new_re_scope_set.element_set()
        names_to_preserve = original_names - originals_renamed
        complete_re_scope = old_to_new_re_scope_set | names_to_preserve
        return self.re_scope(complete_re_scope)

That’s passing. Do we prefer it to this?

    def rename(self, re_scoping_set: Self):
        # renames this set, not its contents sets
        old_names = self.scope_set()
        re_scoping_set = re_scoping_set - old_names
        replaced_names = re_scoping_set.element_set()
        update = replaced_names | re_scoping_set
        renames = old_names ^ update
        return self.re_scope(renames)

I would say the new one is more clear. It’s one line shorter and doesn’t use symmetric difference, which was of course the clever idea back in the old article. Too clever.

We’ll commit this: more direct implementation of rename.

Can we do better? Our intention is that the re-scoping set gets to rename as many things as it wants, and it can in fact produce duplicates by renaming twice. But, for anything not mentioned in the re-scoping set, we want to keep the original.

I don’t see anything better. Will think on it.

Well spotted Laurent! Thanks for reading! I hope you’re not alone, my only reader.

See you next time!