icatproject/python-icat

Invalid return value from the formal string representation operator Query.__repr__()

Opened this issue · 1 comments

The internal data structures in the attributes of class Query are going to be changed in #89. This has an impact on the formal string representation operator Query.__repr__().

According to Python standards, the return value of the formal string representation operator should “look like a valid Python expression that could be used to recreate an object with the same value”. In the current implementation, Query.__repr__() simply returns the internal attribute values. As a result of the changes in #89 these values are not suitable to be passed as arguments to the Query() constructor any more.

Examples:

>>> query = Query(client, "Investigation", conditions={"name": "= '10100601-ST'"})
>>> str(query) 
"SELECT o FROM Investigation o WHERE o.name = '10100601-ST'"
>>> repr(query)
'Query(<icat.client.Client object at 0x7f00a26d7e50>, \'Investigation\', attributes=[], aggregate=None, order=OrderedDict(), conditions={\'name\': ["%s = \'10100601-ST\'"]}, includes=set(), limit=None)'
>>> # take the output of repr() to recreate the query:
>>> q = Query(client, 'Investigation', attributes=[], aggregate=None, order=OrderedDict(), conditions={'name': ["%s = '10100601-ST'"]}, includes=set(), limit=None)
>>> str(q)
"SELECT o FROM Investigation o WHERE o.name %s = '10100601-ST'"

>>> query = Query(client, "Datafile", order=[("name", "DESC")])
>>> str(query)
'SELECT o FROM Datafile o ORDER BY o.name DESC'
>>> repr(query)
"Query(<icat.client.Client object at 0x7f00a26d7e50>, 'Datafile', attributes=[], aggregate=None, order=OrderedDict([('name', '%s DESC')]), conditions={}, includes=set(), limit=None)"
>>> q = Query(client, 'Datafile', attributes=[], aggregate=None, order=OrderedDict([('name', '%s DESC')]), conditions={}, includes=set(), limit=None)
>>> str(q)
'SELECT o FROM Datafile o ORDER BY o.name'

>>> query = Query(client, "User", conditions={ "LENGTH(fullName)": "> 11" })
>>> str(query)
'SELECT o FROM User o WHERE LENGTH(o.fullName) > 11'
>>> repr(query)
"Query(<icat.client.Client object at 0x7f00a26d7e50>, 'User', attributes=[], aggregate=None, order=OrderedDict(), conditions={'fullName': ['LENGTH(%s) > 11']}, includes=set(), limit=None)"
>>> q = Query(client, 'User', attributes=[], aggregate=None, order=OrderedDict(), conditions={'fullName': ['LENGTH(%s) > 11']}, includes=set(), limit=None)
>>> str(q)
'SELECT o FROM User o WHERE o.fullName LENGTH(%s) > 11'

I can essentially see three options to deal with this:

  1. fix __repr__() reconstructing suitable values that can be passed as arguments to Query(),
  2. ignore the issue condoning that the return value of repr() is not suitable to recreate the query object,
  3. deprecate Query.__repr__() and remove it in version 1.0.

Option 1 would formally be the best solution. It would be certainly possible but quite involved. Option 3 would also be formally correct but quite drastic. Query.__repr__() can be a quite useful debugging tool and I'd assume it is in practice only used for debugging and never user to recreate new query objects from the output. That would be an argument for option 2.

As discussed in today's monthly meeting, we won't fix this for the upcoming release 0.20.0. Nevertheless, I leave it open for the moment, maybe we'll fix it later. Maybe not.