DelayedBase.__repr__ does not use repr?

[albertz]

sisyphus/sisyphus/delayed_ops.py

Line 77 in a5de7fe

return str(self.get())

class DelayedBase:
    ...

    def __repr__(self):
        return str(self.get())

    def __str__(self):
        return str(self.get())

Why is there str(...) for the __repr__? Isn't this inconsistent? Shouldn't it be repr?

[critias]

I agree, it looks like a copy&paste bug. Fixing it could cause to break some recipes 🤔 but I guess it should be done anyway.

[michelwi]

I think if we decided to break stuff, then we should put an effort to make the behavior of DelayedBase and other DelayedOps consistent to those of Path

sisyphus/sisyphus/job_path.py

Lines 260 to 264 in 5e8468d

	def __repr__(self):
	if gs.LEGACY_PATH_CONVERSION:
	return repr(str(self))
	else:
	return '<Path %s>' % self.get_path()

and Variable

sisyphus/sisyphus/job_path.py

Lines 443 to 450 in 5e8468d

	def __repr__(self):
	if gs.LEGACY_VARIABLE_CONVERSION:
	return repr(self.get())
	else:
	value = ''
	if self.is_set():
	value = ' %s' % self.get()
	return "<Variable %s%s>" % (self.rel_path(), value)

i.e. the __str__ function serializes the object to its usable form, but may only be used by a worker. the __repr__ may also be used in the manager for logging or displaying on the gui, but should be unusable "by accident" to prevent hash-bugs.

Also we should be more rigorous in applying the check_is_worker function.

Fixing it could cause to break some recipes

It will definitively break things in i6_core, but a am willing to spend time to fix this. It has cost us a lot of time tracing down dubious bugs in the past.