Add user manual for observe by kitchoi · Pull Request #1060 · enthought/traits

kitchoi · 2020-05-01T16:20:35Z

This PR add user manual for the observe framework.

The documentation for on_trait_change is moved to listening.rst. The first paragraph is extracted as an intermediate index page for notifications.
Add observing.rst for the observe framework.
Add traits.observation.api to the API reference for cross-referencing.
Add examples to examples/tutorials/doc_examples

Edited (this was the previous message:)
~~This is a draft PR for making the observe user manual visible (see #977).~~

Much of the API cross-references are missing in this PR, as the PR would otherwise be too big. To see the documentation with styling and cross-referencing, checkout observer-framework-test branch and build the documentation using python etstool.py docs.

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~~

corranwebster · 2020-05-21T07:25:45Z

+Requesting trait attribute change notifications can be done in several
+ways:
+
+.. index:: notification; strategies
+
+* Dynamically, by calling on_trait_change() or on_trait_event() to establish
+  (or remove) change notification handlers.
+* Statically, by decorating methods on the class with the @on_trait_change
+  decorator to indicate that they handle notification for specified attributes.
+* Statically, by using a special naming convention for methods on the class to
+  indicate that they handle notifications for specific trait attributes.
+
+.. index:: notification; dynamic


This probably needs to be updated to give the context for the older methods of notification.

corranwebster · 2020-05-21T07:27:07Z

+  decorator to indicate that they handle notification for specified attributes.
+* Via instance method: By calling |HasTraits.observe| instance method to establish (or remove) change
+  notification handlers.
+


You might mention listeners somewhere here as an older alternative.

corranwebster · 2020-05-21T07:28:32Z

+applying the |@observe| decorator on an instance method. It has the following
+signature:
+
+.. py:decorator:: observe(expression[, post_init=False, dispatch="same"])


Can this be made to use autodoc so that it doesn't get out of date if we change the API?

autodoc ends up bringing the entire docstring here, which is too verbose, an alternative is to write our custom hooks for autodoc but that is probably an overkill. I agree with your concern, so I removed these now and hopefully the links to the API documentation is enough. I reworked these paragraphs to provide the same level of context for the subsequent sections. In particular, the mention of "expression" and "handler" are needed for later sections.

corranwebster · 2020-05-21T07:29:13Z

+The |HasTraits.observe| method is useful for adding or removing change handler on a per
+instance basis. The method has the following signature:
+
+.. py:method:: HasTraits.observe(handler, expression[, remove=False, dispatch="same"])


Same comment about autodoc here

corranwebster · 2020-05-21T07:42:39Z

+Notification Handler
+--------------------
+
+As soon as a change has occurred, the **handler** callable provided will be


"As soon as" is vague. We should probably say something about:

dispatch usually being immediate, but dependent on the dispatch keyword (eg. dispatch="ui" is not immediate)

the order that handlers are called being arbitrary.

corranwebster · 2020-05-21T07:43:59Z

+- handler(*event*)
+
+where the *event* parameter is an object representing the change observed.
+The type of *event* depends on the context of the change.


We should mention here any guarantees that we make about the common API, eg. if object is always present.

corranwebster · 2020-05-21T07:44:43Z

+.. rubric:: Notes
+
+#. When a handler is invoked, the change has already occurred.
+#. Multiple handlers can be defined for a given change event and these handlers


I'd bump this up, rather than have it in a note (see previous comments).

corranwebster · 2020-05-21T07:45:27Z

+   captured and logged.
+
+
+Differences from |@on_trait_change|


Maybe "migrating from on_trait_change"?

corranwebster

A couple of further comments:

the example code should have module docstrings so that the TraitsUI demo code can display it properly
while the content is good, we may want to restructure how it is presented into:
- how observation works without any significant reference to listeners (maybe mention their existence in an introduction)
- a more buried section on listeners (part of an "advanced" section, or an appendix) which indicates that it is an older way of doing things
- a "migration guide" section which includes things like differences between listeners and observers, and later things like moving from Either to Union and handling mapped traits with Map

…t is an older mechanism

kitchoi · 2020-05-21T16:48:19Z

Thank you @corranwebster, all very good suggestions and I made quite a lot of changes:

Merged ‘observing’ and ‘notification’ so that observe is the main content for ‘Traits Notification’.
on_trait_change is moved to an Appendix and is referenced in the introduction for observe.
Regrouped some of the “differences from on_trait_change” into a section more like a migration guide
Rebranded some of the “differences from on_trait_change” as features/fixes, with as little mention of on_trait_change as possible.
Moved the expectations/rules about the change handler up
Mentioned the “dispatch” parameter and its effect on handler
Simplified migration-guide-like sections to make them easier to read at a glance.

kitchoi · 2020-05-21T16:49:23Z

+introduced for overcoming them. See :ref:`observe-notification` for details and
+for how to migrate.
+
+Change notifications can be set up with **on_trait_change** in several ways:


This sentence and the paragraph above are changed. The rest are the same as the original documentation.

kitchoi · 2020-05-21T16:52:27Z

I overlooked this important request change:

the example code should have module docstrings so that the TraitsUI demo code can display it properly

Will continue making changes for this. Removing review request for now. I am sorry for the noise.

Edited: Done.

mdickinson · 2020-05-22T09:15:59Z

+
+   * - Expression
+     - Meaning
+   * - *trait("attr1")*


Can we add backticks around these, so that the rendered expressions can be copy-pasted? (Currently they're appearing with code-unfriendly smart quotes.)

They will look rather long and squash the "meaning" column into a small column.
I could instead break this out into bulletins instead of trying to fit them in a table.
Happy to do this in a separate PR so not to block the release process.

That could work. Right now, the rendered code can't be copy-and-pasted because of the smart quotes.

I thought it was going to take me a while to get the formatting right. It did not take so long at the end: See #1140

mdickinson · 2020-05-22T09:16:55Z

This LGTM. There are some places where the wording could be tweaked, but I'm happy for that to happen in a separate PR.

kitchoi · 2020-05-22T09:28:06Z

If this is good enough to go in, let's do tweaks in separate PRs so we don't block release preparations.

corranwebster

Comments are in examples, so I am OK with it being merged.

kitchoi · 2020-05-22T09:43:56Z

Thank you both! Merging...
I am working on changes for addressing #1060 (comment)
That will be in a separate PR.

rahulporuri · 2020-05-22T10:41:55Z

+     - Meaning
+   * - *attr1\.attr2*
+     - Matches a trait named *attr2* on an object referenced by a trait named
+       *item1* on the current object. Changes to *attr1* or *attr2* will


should this be "attr1", not "item1"

Well spotted! Will fix.

kitchoi added 3 commits May 1, 2020 17:14

Add draft user manual for observe

b172746

Add the example scripts

d62c90a

Fix a missing negation

a36fb9c

corranwebster reviewed May 4, 2020

View reviewed changes

Comment thread docs/source/traits_user_manual/observers.rst Outdated

corranwebster reviewed May 4, 2020

View reviewed changes

Comment thread docs/source/traits_user_manual/observers.rst Outdated

corranwebster reviewed May 4, 2020

View reviewed changes

Comment thread docs/source/traits_user_manual/observers.rst Outdated

This was referenced May 5, 2020

Maint: Avoid calling observe's dispatch with named arguments #1065

Merged

Add TraitChangeEvent object for wrapping CTrait change #1070

Merged

mdickinson added the component: documentation Issues related to the Sphinx documentation label May 11, 2020

kitchoi added 7 commits May 20, 2020 16:16

Merge branch 'master' into doc-observe-user-manual

ccef7d6

Reformat documentation following a lot of renaming

ff774f4

Update examples for observe

19365cb

Fix comparison mode

ff5b943

Mention comparison mode

1868795

Fix parameter documentation

076be53

More minor improvement to the documentation

074344e

kitchoi changed the title ~~[WIP] Add draft user manual for observe~~ Add user manual for observe May 20, 2020

kitchoi marked this pull request as ready for review May 20, 2020 17:51

corranwebster reviewed May 21, 2020

View reviewed changes

kitchoi mentioned this pull request May 21, 2020

Implement new framework to observe and notify for trait changes #977

Closed

19 tasks

mdickinson mentioned this pull request May 21, 2020

Release notes and changelog for 6.1.0 #1133

Merged

Edit the first section for on_trait_change to mention the fact that i…

dbfd92e

…t is an older mechanism

kitchoi added 9 commits May 21, 2020 17:26

Add a similar paragraph for the introduction in observe

65c981f

Merge notification and observing

8746a3a

Minor improvement to a new sentence.

a66bd62

Move documentation on_trait_change to Appendix

c9f9dc5

Improve example print_status

e4a2e27

Modify a section name

863a7e0

Fix formatting

bf9a0ae

Replace duplicated word

f99f317

More formatting improvement

6678740

kitchoi requested a review from corranwebster May 21, 2020 16:48

kitchoi commented May 21, 2020

View reviewed changes

kitchoi removed the request for review from corranwebster May 21, 2020 16:51

kitchoi added 3 commits May 21, 2020 17:55

Use markers that are more likely to stay the same

d20904b

Add docstrings on examples so that they are shown in the demo app

3a06d75

Nit: Minor language improvement

db63835

kitchoi requested a review from corranwebster May 21, 2020 17:10

Minor fix to grammar

4b8fa5e

mdickinson reviewed May 22, 2020

View reviewed changes

mdickinson approved these changes May 22, 2020

View reviewed changes

mdickinson mentioned this pull request May 22, 2020

"duplicate object description" warnings in documentation build #1045

Closed

corranwebster approved these changes May 22, 2020

View reviewed changes

kitchoi merged commit d60b380 into master May 22, 2020

kitchoi deleted the doc-observe-user-manual branch May 22, 2020 09:44

rahulporuri reviewed May 22, 2020

View reviewed changes

kitchoi mentioned this pull request May 22, 2020

Fix attribute name in documentation #1143

Merged

1 task

Conversation

kitchoi commented May 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

corranwebster left a comment

Choose a reason for hiding this comment

Uh oh!

kitchoi commented May 21, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kitchoi commented May 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdickinson commented May 22, 2020

Uh oh!

kitchoi commented May 22, 2020

Uh oh!

corranwebster left a comment

Choose a reason for hiding this comment

Uh oh!

kitchoi commented May 22, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kitchoi commented May 1, 2020 •

edited

Loading

kitchoi commented May 21, 2020 •

edited

Loading