Skip to content

Minor stylistic changes to demos in Standard_Editors#932

Merged
ievacerny merged 4 commits into
masterfrom
maint/866-minor-standard-editor-demo-fixes
Jul 1, 2020
Merged

Minor stylistic changes to demos in Standard_Editors#932
ievacerny merged 4 commits into
masterfrom
maint/866-minor-standard-editor-demo-fixes

Conversation

@ievacerny
Copy link
Copy Markdown
Contributor

@ievacerny ievacerny commented Jun 29, 2020

Addresses #866

These are mostly flake8, formatting and other minor stylistic fixes for demos in Standard_Editors.

Some of the demos are broken. Warnings will be added soon, after relevant issues are opened. Warning are added to demos that are broken in some way. Issue references also added as comments.

More important changes are pointed out with in-code review comments.

@ievacerny ievacerny self-assigned this Jun 29, 2020
traits_view = View(
'my_button_trait',
'click_counter',
Item('click_counter', style='readonly'),
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.

This was changed to avoid #928

Comment on lines +21 to +24
compound_trait = Union(
Range(1, 6), Enum('a', 'b', 'c', 'd', 'e', 'f'),
default_value=1
)
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'm not too sure about this change - is it too soon? The demo won't work with Traits<6.1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It might be a bit harsh to have the script fails with a nasty exception when the user's traits version is less than 6.1, especially since TraitsUI hasn't changed its install requirement to Traits >= 6.1 yet (I think).

We could do something like this I guess:

def somename():
    try:
        from traits.api import Union
    except ImportError:
        # traits < 6.1
        from traits.api import Either
        return Either(Range(1, 6), Enum("a", "b", ...), default=1)
   else:
        return Union(Range(1, 6), Enum("a", "b", ...), default_value=1)

Would be a good example to highlight the difference between the two. The downside though is that one of the branches won't be tested with CI.
Alternatively, keep Either and add a comment about Union being better but require Traits > 6.1

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 adding such a function might be too complex for simple demonstration of an editor, especially for people who are just starting out with Python and Traits. I changed it to Either and added a comment to inform about Union.

Comment on lines +91 to +103
if self.value >= 0:
return (
'The square root of {} is {}'.format(
self.value, self.value ** 0.5
)
)
else:
return (
'The square root of {} is {}i'.format(
self.value, (-self.value) ** 0.5
)
)

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.

Raising negative numbers to fractional power no longer raises an exception

Comment on lines -38 to -39
# FIXME: provide accessible copy or equivalent of factories_advanced_extra.rst

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 don't think this is needed for demos. Above referral to User Manual should be enough to find relevant information.

Item('image_from_list', style='readonly', label='ReadOnly')
Item('image_from_list', style='readonly', label='ReadOnly'),
Item('_'),
Item('image_from_list', style='custom', label='Custom')
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.

With wx toolkit, the popup from simple style editor would blend with the custom editor and it looked like the simple editor was broken. To avoid this I moved the custom style to the bottom of the window.

Copy link
Copy Markdown
Contributor

@kitchoi kitchoi left a comment

Choose a reason for hiding this comment

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

LGTM

@ievacerny ievacerny merged commit bb91dbb into master Jul 1, 2020
@ievacerny ievacerny deleted the maint/866-minor-standard-editor-demo-fixes branch July 1, 2020 16:41
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