Alternative approach to handling equality comparison mode for notifications

[kitchoi]

This PR is currently a draft for discussion purposes.

This PR presents an alternative approach to support observe even when the List, Set and Dict trait types have comparison mode set to equality.

The solution is to move the rich comparison from CTrait to the notifier, such that instead of silencing the notifications early in CTrait, the notifiers will do the equality check and silence the notification if applicable.

Pros:

• ComparisonMode.equality has the desired effect on trait notifications via both on_trait_change and observe, i.e. no need for Migration to identity comparison mode for container traits #1122

Cons:

• The equality check is done as many times as the number of notifiers attached to a trait. I hope this only presents a very slight performance cost.

Side-effect:

• Previously post_setattr (if defined) is not called if the notifications are silenced due to equality check. Now post_setattr will be called even if change notifications are silenced due to equality check. The original behaviour looks to me an optimization that can be removed, but I am not entirely sure.

Uncertainty:

• It looks as though the code path that reaches rich comparison in ctraits.c only applies to trait types with TraitType.trait. e.g. notifications emitted for Property and Delegate do not go through these equality checks. Filtering for TraitType.trait in the notifiers look enough, but I am not sure if there are other code paths using this rich comparison that have not been accounted for.

This approach may be an alternative to #1122.

If this works, it should present no visible changes to users of traits. However I think this is quite a substantial internal change that presents quite a bit of risk. Hence I'd be hesitant to let this into Traits 6.1 (it won't make it in anyway judging from the time line). It is perhaps reasonable for observe to warn about equality mode and leave CTrait/on_trait_change untouched in at least one release. (Edit: 6.1 is now out 🎉 )

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

[mdickinson]

I think this is the right approach. A couple of minor comments.

[kitchoi]

Updated main comment: User manual is also updated now.

[mdickinson]

Changes look good, except that I think we simply want to update the documentation to match the Traits 6.2 behaviour rather than describing what happened in 6.1. (And a couple of tiny markup nitpicks.)

[mdickinson]

I think we simply want to update the documentation to match the Traits 6.2 behaviour rather than describing what happened in 6.1.

We could possibly make use of the .. versionchanged:: ... markup to add a note about the change from 6.1 to 6.2, though.

[kitchoi]

We could possibly make use of the .. versionchanged:: ... markup to add a note about the change from 6.1 to 6.2, though.

I have removed the section. Maybe the change can later live in the changelog / release notes?

[mdickinson]

LGTM! Waiting for CI ...