row_factory should be a callable for TableEditors

[aaronayres35]

Checklist

• Tests
• [ ] Update API reference (docs/source/traits_api_reference)
• [ ] Update User manual (docs/source/traits_user_manual)
• [ ] Update type annotation hints in traits-stubs

This PR fixes enthought/traitsui#1539
The row_factory for a TableEditor should be a callable if it is not None, see:
https://github.com/enthought/traitsui/blob/7a6d46ad10ea6c6b5a8dcc3b4939a9670c2a52ad/traitsui/editors/table_editor.py#L291-L296

Previously there was a case in which we used an Instance trait's default_value. However, there is a situation in which this is an instance of _InstanceArgs, see:

traits/traits/trait_types.py

Lines 3353 to 3356 in 2b62cd9

	else:
	value = _InstanceArgs(factory, args, kw)
	self.default_value = value

EDIT: This was not correct, it was actually just a tuple coming from

traits/traits/trait_types.py

Lines 3420 to 3437 in 5f2effc

	def get_default_value(self):
	""" Returns a tuple of the form: ( default_value_type, default_value )
	which describes the default value for this trait.
	"""
	dv = self.default_value
	dvt = self.default_value_type
	if dvt < 0:
	if not isinstance(dv, _InstanceArgs):
	return super().get_default_value()
	self.default_value_type = dvt = DefaultValue.callable_and_args
	self.default_value = dv = (
	self.create_default_value,
	dv.args,
	dv.kw,
	)
	return (dvt, dv)

where dv was an _InstanceArgs instance. The tuple is of the form (self.create_default_value, dv.args, dv.kw) so ultimately we still do want to have a change similar to the one shown below:

This PR avoids that problem scenario by checking and making the necessary adjustment if needed. Similar to what is done here:

traits/traits/trait_types.py

Lines 3394 to 3399 in 2b62cd9

	else:
	result = self.default_value
	if isinstance(result, _InstanceArgs):
	return result[0](*result[1], **result[2])
	else:
	return result

[aaronayres35]

I changed this line as I thought list_editor(trait, trait) was more confusing to read, but I now remember i had simply copied it from the other tests. I am happy to revert if that is preferred

[rahulporuri]

LGTM. I tested this fix against the bug report enthought/traitsui#1538 - and this fix works as expected. Let's still wait for @mdickinson s review.

[mdickinson]

Thanks for this; will aim to review this week.

[mdickinson]

I'm happy to go with @rahulporuri's LGTM here. This is really TraitsUI code that happens to live in Traits for historical reasons.

[mdickinson]

One suggestion here: checking handler.default_value_type should be more robust than doing the isinstance check.

[mdickinson]

Suggestion: instead of this isinstance check, check whether handler.default_value_type is equal to DefaultValue.callable_and_args.

[mdickinson]

I'm happy to go with @rahulporuri's LGTM here.

Sorry, I'm rolling that back. @aaronayres35: please could you change the code to dispatch based on the value of handler.default_value_type instead of using the isinstance check?