Skip to content

Remove events #550

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jan 25, 2019
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,7 @@ jobs:
command: |
. venv/bin/activate
python --version
python -m unittest tests.development.test_base_component
python -m unittest tests.development.test_component_loader
python -m unittest tests.test_integration
python -m unittest tests.test_resources
python -m unittest tests.test_configs
./test.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

👍


"python-3.6":
<<: *test-template
Expand All @@ -80,4 +76,4 @@ workflows:
jobs:
- "python-2.7"
- "python-3.6"
- "python-3.7"
- "python-3.7"
2 changes: 1 addition & 1 deletion .circleci/requirements/dev-requirements-py37.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ dash_core_components>=0.40.2
dash_html_components==0.12.0rc3
dash-flow-example==0.0.3
dash-dangerously-set-inner-html
dash_renderer
git+git://github.com/plotly/dash-renderer@no-events#egg=dash_renderer
percy
selenium
mock
Expand Down
2 changes: 1 addition & 1 deletion .circleci/requirements/dev-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ dash_core_components>=0.40.2
dash_html_components>=0.12.0rc3
dash_flow_example==0.0.3
dash-dangerously-set-inner-html
dash_renderer
git+git://github.com/plotly/dash-renderer@no-events#egg=dash_renderer
percy
selenium
mock
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## Unreleased
## Removed
- Removed support for `Event` system. Use event properties instead, for example the `n_clicks` property instead of the `click` event, see [#531](https://github.com/plotly/dash/issues/531) for details. `dash_renderer` MUST be upgraded to >=0.17.0 together with this, and it is recommended to update `dash_core_components` to >=0.43.0 and `dash_html_components` to >=0.14.0. [#550](https://github.com/plotly/dash/pull/550)

## [0.35.3] - 2019-01-23
## Fixed
- Asset blueprint takes routes prefix into it's static path. [#547](https://github.com/plotly/dash/pull/547)
Expand Down
46 changes: 14 additions & 32 deletions dash/dash.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from flask import Flask, Response
from flask_compress import Compress

from .dependencies import Event, Input, Output, State
from .dependencies import Input, Output, State
from .resources import Scripts, Css
from .development.base_component import Component
from . import exceptions
Expand Down Expand Up @@ -622,7 +622,6 @@ def dependencies(self):
},
'inputs': v['inputs'],
'state': v['state'],
'events': v['events']
} for k, v in self.callback_map.items()
])

Expand All @@ -633,7 +632,7 @@ def react(self, *args, **kwargs):
'Use `callback` instead. `callback` has a new syntax too, '
'so make sure to call `help(app.callback)` to learn more.')

def _validate_callback(self, output, inputs, state, events):
def _validate_callback(self, output, inputs, state):
# pylint: disable=too-many-branches
layout = self._cached_layout or self._layout_value()

Expand All @@ -652,8 +651,7 @@ def _validate_callback(self, output, inputs, state, events):

for args, obj, name in [([output], Output, 'Output'),
(inputs, Input, 'Input'),
(state, State, 'State'),
(events, Event, 'Event')]:
(state, State, 'State')]:

if not isinstance(args, list):
raise exceptions.IncorrectTypeException(
Expand Down Expand Up @@ -721,32 +719,20 @@ def _validate_callback(self, output, inputs, state, events):
component.available_properties).replace(
' ', ''))

if (hasattr(arg, 'component_event') and
arg.component_event not in
component.available_events):
if hasattr(arg, 'component_event'):
raise exceptions.NonExistentEventException('''
Attempting to assign a callback with
the event "{}" but the component
"{}" doesn't have "{}" as an event.\n
Here is a list of the available events in "{}":
{}
'''.format(
arg.component_event,
arg.component_id,
arg.component_event,
arg.component_id,
component.available_events).replace(' ', ''))
Events have been removed.
Use the associated property instead.
''')

if state and not events and not inputs:
raise exceptions.MissingEventsException('''
if state and not inputs:
raise exceptions.MissingInputsException('''
This callback has {} `State` {}
but no `Input` elements or `Event` elements.\n
Without `Input` or `Event` elements, this callback
but no `Input` elements.\n
Without `Input` elements, this callback
will never get called.\n
(Subscribing to input components will cause the
callback to be called whenever their values
change and subscribing to an event will cause the
callback to be called whenever the event is fired.)
callback to be called whenever their values change.)
'''.format(
len(state),
'elements' if len(state) > 1 else 'element'
Expand Down Expand Up @@ -888,8 +874,8 @@ def _validate_value(val, index=None):
# TODO - Check this map for recursive or other ill-defined non-tree
# relationships
# pylint: disable=dangerous-default-value
def callback(self, output, inputs=[], state=[], events=[]):
self._validate_callback(output, inputs, state, events)
def callback(self, output, inputs=[], state=[]):
self._validate_callback(output, inputs, state)

callback_id = '{}.{}'.format(
output.component_id, output.component_property
Expand All @@ -902,10 +888,6 @@ def callback(self, output, inputs=[], state=[], events=[]):
'state': [
{'id': c.component_id, 'property': c.component_property}
for c in state
],
'events': [
{'id': c.component_id, 'event': c.component_event}
for c in events
]
}

Expand Down
7 changes: 0 additions & 7 deletions dash/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,3 @@ class State:
def __init__(self, component_id, component_property):
self.component_id = component_id
self.component_property = component_property


# pylint: disable=old-style-class, too-few-public-methods
class Event:
def __init__(self, component_id, component_event):
self.component_id = component_id
self.component_event = component_event
47 changes: 15 additions & 32 deletions dash/development/_py_components_generation.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ def generate_class_string(typename, props, description, namespace):
string

"""
# TODO _prop_names, _type, _namespace, available_events,
# and available_properties
# TODO _prop_names, _type, _namespace, and available_properties
# can be modified by a Dash JS developer via setattr
# TODO - Tab out the repr for the repr of these components to make it
# look more like a hierarchical tree
Expand All @@ -52,7 +51,6 @@ def __init__(self, {default_argtext}):
self._namespace = '{namespace}'
self._valid_wildcard_attributes =\
{list_of_valid_wildcard_attr_prefixes}
self.available_events = {events}
self.available_properties = {list_of_valid_keys}
self.available_wildcard_properties =\
{list_of_valid_wildcard_attr_prefixes}
Expand Down Expand Up @@ -101,11 +99,11 @@ def __repr__(self):
docstring = create_docstring(
component_name=typename,
props=filtered_props,
events=parse_events(props),
description=description).replace('\r\n', '\n')

prohibit_events(props)

# pylint: disable=unused-variable
events = '[' + ', '.join(parse_events(props)) + ']'
prop_keys = list(props.keys())
if 'children' in props:
prop_keys.remove('children')
Expand All @@ -122,7 +120,7 @@ def __repr__(self):
for p in prop_keys
if not p.endswith("-*") and
p not in python_keywords and
p not in ['dashEvents', 'fireEvent', 'setProps']] + ['**kwargs']
p != 'setProps'] + ['**kwargs']
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if that was intentional but still present for the R generation

Copy link
Contributor

@rpkyle rpkyle Jan 21, 2019

Choose a reason for hiding this comment

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

Have modified my code to match.

)

required_args = required_props(props)
Expand Down Expand Up @@ -233,7 +231,7 @@ def required_props(props):
if prop['required']]


def create_docstring(component_name, props, events, description):
def create_docstring(component_name, props, description):
"""
Create the Dash component docstring

Expand All @@ -243,8 +241,6 @@ def create_docstring(component_name, props, events, description):
Component name
props: dict
Dictionary with {propName: propMetadata} structure
events: list
List of Dash events
description: str
Component description

Expand All @@ -259,9 +255,7 @@ def create_docstring(component_name, props, events, description):
return (
"""A {name} component.\n{description}

Keyword arguments:\n{args}

Available events: {events}"""
Keyword arguments:\n{args}"""
).format(
name=component_name,
description=description,
Expand All @@ -274,30 +268,24 @@ def create_docstring(component_name, props, events, description):
description=prop['description'],
indent_num=0,
is_flow_type='flowType' in prop and 'type' not in prop)
for p, prop in list(filter_props(props).items())),
events=', '.join(events))
for p, prop in list(filter_props(props).items())))


def parse_events(props):
def prohibit_events(props):
"""
Pull out the dashEvents from the Component props
Events have been removed. Raise an error if we see dashEvents or fireEvents

Parameters
----------
props: dict
Dictionary with {propName: propMetadata} structure

Returns
Raises
-------
list
List of Dash event strings
?
"""
if 'dashEvents' in props and props['dashEvents']['type']['name'] == 'enum':
events = [v['value'] for v in props['dashEvents']['type']['value']]
else:
events = []

return events
if 'dashEvents' in props or 'fireEvents' in props:
raise AttributeError('Events are no longer supported by dash')
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a custom exception or raise a DeprecationWarning instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, it wasn't deprecated, it was outright removed 😅 but I'm happy to make a custom exception - ObsoleteEventsError?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think exceptions.NonExistantPropException with a event removal message would fit the bill.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I used NonExistentEventException, which is the same I use for corresponding obsolete callback usage.



def parse_wildcards(props):
Expand Down Expand Up @@ -349,7 +337,6 @@ def filter_props(props):
Filter props from the Component arguments to exclude:
- Those without a "type" or a "flowType" field
- Those with arg.type.name in {'func', 'symbol', 'instanceOf'}
- dashEvents as a name

Parameters
----------
Expand Down Expand Up @@ -415,10 +402,6 @@ def filter_props(props):
else:
raise ValueError

# dashEvents are a special oneOf property that is used for subscribing
# to events but it's never set as a property
if arg_name in ['dashEvents']:
filtered_props.pop(arg_name)
return filtered_props


Expand Down Expand Up @@ -518,7 +501,7 @@ def map_js_to_py_types_prop_types(type_object):
', '.join(
"'{}'".format(t)
for t in list(type_object['value'].keys())),
'Those keys have the following types: \n{}'.format(
'Those keys have the following types:\n{}'.format(
'\n'.join(create_prop_docstring(
prop_name=prop_name,
type_object=prop,
Expand Down Expand Up @@ -561,7 +544,7 @@ def map_js_to_py_types_flow_types(type_object):
signature=lambda indent_num: 'dict containing keys {}.\n{}'.format(
', '.join("'{}'".format(d['key'])
for d in type_object['signature']['properties']),
'{}Those keys have the following types: \n{}'.format(
'{}Those keys have the following types:\n{}'.format(
' ' * indent_num,
'\n'.join(
create_prop_docstring(
Expand Down
2 changes: 1 addition & 1 deletion dash/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class IncorrectTypeException(CallbackException):
pass


class MissingEventsException(CallbackException):
class MissingInputsException(CallbackException):
pass


Expand Down
2 changes: 1 addition & 1 deletion dash/extract-meta.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const componentPaths = process.argv.slice(3);
const ignorePattern = new RegExp(process.argv[2]);

const excludedDocProps = [
'setProps', 'id', 'className', 'style', 'dashEvents', 'fireEvent'
'setProps', 'id', 'className', 'style'
];

if (!componentPaths.length) {
Expand Down
15 changes: 15 additions & 0 deletions test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
EXIT_STATE=0

python -m unittest tests.development.test_base_component || EXIT_STATE=$?
python -m unittest tests.development.test_component_loader || EXIT_STATE=$?
python -m unittest tests.test_integration || EXIT_STATE=$?
python -m unittest tests.test_resources || EXIT_STATE=$?
python -m unittest tests.test_configs || EXIT_STATE=$?

if [ $EXIT_STATE -ne 0 ]; then
echo "One or more tests failed"
else
echo "All tests passed!"
fi

exit $EXIT_STATE
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we include the linting also ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure! I'll move linting in here too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

linting into test.sh -> 2c9a3c8

10 changes: 0 additions & 10 deletions tests/development/TestReactComponent.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,19 +91,9 @@ ReactComponent.propTypes = {
}
}),

// special dash events

children: React.PropTypes.node,

id: React.PropTypes.string,


// dashEvents is a special prop that is used to events validation
dashEvents: React.PropTypes.oneOf([
'restyle',
'relayout',
'click'
])
};

ReactComponent.defaultProps = {
Expand Down
21 changes: 0 additions & 21 deletions tests/development/metadata_test.json
Original file line number Diff line number Diff line change
Expand Up @@ -234,27 +234,6 @@
},
"required": false,
"description": ""
},
"dashEvents": {
"type": {
"name": "enum",
"value": [
{
"value": "'restyle'",
"computed": false
},
{
"value": "'relayout'",
"computed": false
},
{
"value": "'click'",
"computed": false
}
]
},
"required": false,
"description": ""
}
}
}
Loading