SynBioDex / pySBOL3

Native python implementation of SBOL 3.0 specification
MIT License
37 stars 16 forks source link

Add dunder for better representation #377

Closed mohitdmak closed 2 years ago

mohitdmak commented 2 years ago

Hi @jakebeal , I was going through this repo to better understand the implementation of the sbol3 standard here which is used in sbol utilities, and tried to fix #303 . But I noticed that printing sbol.Component objects gave a proper output as you described in the latter eg of the issue, however just entering the component object itself, the former non-informative version gets printed. So adding a __repr__() method to Identified seems to fix this, is this alright?

tcmitchell commented 2 years ago

Hi @mohitdmak , thanks for the contribution. I am hesitant to merge this pull request. I use the existing representation of the objects for debugging purposes and the <sbol3.component.Component object at 0x7fec4006af10> type strings help me know which object I'm looking at and whether it's different from some other object. Your modification here would make all objects with the same identity look identical, and indistinguishable.

The [python documentation for repr] says:

This is typically used for debugging, so it is important that the representation is information-rich and unambiguous.

I think your __repr__ method does not provide the unambiguous output that is desirable.

We have the __str__ method and I think it is an oversight that #303 did not get closed when __str__ was added.

@jakebeal what are your thoughts? How do you use the repr vs. the str output?

For reference, here is an interactive session with the current version of sbol3:

>>> import sbol3
>>> sbol3.set_namespace('https://example.org')
>>> c = sbol3.Component('c1', types=[sbol3.SBO_DNA])
>>> c
<sbol3.component.Component object at 0x7fec4006af10>
>>> str(c)
'<Component https://example.org/c1>'
>>> print(c)
<Component https://example.org/c1>
tcmitchell commented 2 years ago

The __str__ method was added in https://github.com/SynBioDex/pySBOL3/pull/365 and was not mentioned there or linked to #303. That's my mistake.

jakebeal commented 2 years ago

I don't see any reason to differ with your assessment here @tcmitchell

mohitdmak commented 2 years ago

Ok sure @tcmitchell , actually I was confused since the issue was open and there existed a str method already in the current version and I thought the representation had to be changed too.