Skip to content

Doumentation improvements and fixes#778

Merged
corranwebster merged 7 commits into
masterfrom
docs/improvements-and-fixes
Apr 3, 2020
Merged

Doumentation improvements and fixes#778
corranwebster merged 7 commits into
masterfrom
docs/improvements-and-fixes

Conversation

@corranwebster
Copy link
Copy Markdown
Contributor

A general set of improvements to the documentation. This adds documentation for a few omitted editors, the UI object, and imporives the description of the UIInfo object, plus assorted minor fixes and improvements.

@mdickinson
Copy link
Copy Markdown
Member

Looks like Travis CI notifications are dodgy. Not sure whether this is random flakiness on the part of GitHub / Travis CI or whether there's some configuration that needs fixing.


The editors generated by ArrayEditor() provide text fields (or static text for
the read-only style) for each cell of a two-dimensional Numeric array. Only the
the read-only style) for each cell of a two-dimensional Numpy array. Only the
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spelling nitpick: NumPy, not Numpy.

*allow_future*, *message*, *months*, *multi_select*, *on_mixed_select*,
*padding*, *shift_to_select*, *selected_style*, *strftime*, *view*

The DateEditor() displays a Python datetime date object, usually supplied via a
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"datetime date" is slightly confusing; represent as datetime.date instead?

The DateEditor() displays a Python datetime date object, usually supplied via a
Date trait. The simple style shows a date spin control, while the custom style
shows one (or potentially more in the wx backend) months in a calendar view.
Dates can be restricted to past-only by setting *allow_future* to False.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had no idea that allow_future was an option. Given that it is, though, it seems strange that allow_past is not. But that's off-topic for this PR.

tweak some of the other parameters without having recreate that data.
The DatetimeEditor() displays a Python datetime object, usually supplied
via a Datetime trait. The simple style shows a datetime spin control, and maximum
and minimum selectable datetimess can be supplied to restrict the range of values.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

datetimessss, preciousss ...

Copy link
Copy Markdown
Member

@mdickinson mdickinson Apr 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry. Ahem. I don't know what came over me there; let me try again. I believe there may be a typographical error in this line. Please consider replacing the word "datetimess" with "datetimes".

class Person ( HasStrictTraits ):
name = Str()
age = Int()
name = Str
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Urgh. Do we really want to do this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that's a regression from previous clean-up. Not sure why that's happening.

:Optional parameters:
*minimum_datetime*, *strftime*

The TimeEditor() displays a Python datetime time object, usually supplied
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as for "datetime date"; I think this is potentially confusing unless it's clear that "datetime" refers to the datetime module.


The TimeEditor() displays a Python datetime time object, usually supplied
via a Time trait. The simple style shows a time spin control. For readonly
style, the user can set the message to display if the datetime value is None,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should "datetime value" be "time value" here?

object. [12]_ For example, the UIInfo object created in
:ref:`Example 7 <example-7-using-a-multi-object-view-with-a-context>`
whose :term:`trait attribute`\ s are displayed in it. This object holds a
reference to the UI instance in it's **ui** trait, and whether the or not
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"it's" -> "its"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's also a spurious "the" between "whether" and "or".

BTW, sorry for not grouping the prior comments together in a review; I'll do that from this point onwards.

Copy link
Copy Markdown
Member

@mdickinson mdickinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; a few nitpicks and questions.

object. [12]_ For example, the UIInfo object created in
:ref:`Example 7 <example-7-using-a-multi-object-view-with-a-context>`
whose :term:`trait attribute`\ s are displayed in it. This object holds a
reference to the UI instance in it's **ui** trait, and whether the or not
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's also a spurious "the" between "whether" and "or".

BTW, sorry for not grouping the prior comments together in a review; I'll do that from this point onwards.

+---------------------------+--------------------------------------------------+
|Method |Purpose |
+===========================+==================================================+
|dispose(result, abort) |Disposes of the UI. This can be called to close a|
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we have spaces around the content in the table entries (as in the examples here)? I don't think it makes a difference to the rendered result, but it makes it more readable in the source.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was following the style of other tables. Can fix all.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you; nitpick level, not important.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I am not going to do it in this PR. There's probably a more thorough overhaul of documentation needed, but not today.

owners.

| Enthought, Inc.
| 200 W Cesar Chavez, Suite 202
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there value in including a postal address here?

@@ -0,0 +1,10 @@
from traits.api import HasTraits, Int
import wx
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we follow the usual import order and put 3rd party imports before ETS imports?

@mdickinson mdickinson closed this Apr 3, 2020
@mdickinson mdickinson reopened this Apr 3, 2020
@mdickinson
Copy link
Copy Markdown
Member

Closing and re-opening in the hope of getting a clean Travis CI run. If it still fails, I'll investigate further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants