copy no longer works

[jakebeal]

With the 1.0b6 release, the copy command no longer works.

Minimal reproducing code:

import sbol3

doc = sbol3.Document()
seq = sbol3.Sequence('http://foo.bar/baz')
doc.add(seq)
doc2 = sbol3.Document()
seq.copy(doc2)
assert len(doc.objects)
assert len(doc2.objects)

The first assertion passes, but the second assertion fails.

[tcmitchell]

I can confirm that the same code works in version 1.0b5 and fails at the current HEAD. There weren't a lot of code changes between the two. It was mainly wrangling dependencies. But there were a few code changes.

Note: There needs to be a test for copy(). That this crept in is clearly a shortcoming of test coverage.

[tcmitchell]

Aha. This is a problem with the semantics of the __len__ method that was added to Document (see #296). (This is also a problem with test coverage.)

The copy method decides whether to add the copy to a document through a boolean conversion of the Document:

        # Assign the new object to the target Document
        if target_doc:
            try:
                target_doc.add(new_obj)
            except TypeError:
                pass  # object is not TopLevel

The documentation for __len__ says (emphasis mine):

Called to implement the built-in function len(). Should return the length of the object, an integer >= 0. Also, an object that doesn’t define a bool() method and whose len() method returns zero is considered to be false in a Boolean context.

The target document in the example in this bug report is empty, when the if target_doc: test is made. Thus len(target_doc) return zero, and "is considered to be false" in this Boolean context. Better test coverage would have caught this before it became a problem.

Since Document.__len__() appears useful, I think we want to discourage the boolean conversion of documents and instead make a proper check like, in this case, if target_doc is not None.