Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

Issue 384 - Handling of None data and columns props #731

Merged
merged 7 commits into from
Apr 20, 2020

Conversation

Marc-Andre-Rivet
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet commented Apr 7, 2020

Closes #384

Changes table validation/sanitation to handle data=None and columns=None without throwing an error.

@Marc-Andre-Rivet Marc-Andre-Rivet marked this pull request as ready for review April 7, 2020 20:29
@chriddyp
Copy link
Member

chriddyp commented Apr 7, 2020

What about if callbacks update an empty datatable dynamically?

app.layout = html.Div([
    html.Label('Select a dataset'),
    dcc.Dropdown(options=[...], value=..., id='dropdown'),
    dash_table.DataTable(id='table')
])

@app.callback(Output('table', 'data'), [Input('dropdown', va'lue')])
def ...

@Marc-Andre-Rivet
Copy link
Contributor Author

Marc-Andre-Rivet commented Apr 7, 2020

@chriddyp Good point. Updated the internal validation/sanitation logic to instead handle that case gracefully.

@Marc-Andre-Rivet Marc-Andre-Rivet changed the title Issue 384 - data and columns are required props Issue 384 - Handling of None data and columns props Apr 7, 2020
Marc-André Rivet added 2 commits April 7, 2020 18:12
- remove default value for data and columns
- sanitize data and columns props
- update columns prop validation
- update TypeScript types
assert target.is_ready()
test.driver.find_element_by_css_selector("#clear-table").click()
assert target.is_ready()
assert len(test.driver.find_elements_by_css_selector("tr")) == 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once the callback returns, the table should (1) load successfully and get to the ready state, (2) should contain no rows.

TDD'ed locally, can undo redo the fix in the PR if we'd rather have the demonstration.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm fine with not seeing the test fail on CI - I assume we're all running our own tests with the fix disabled. But it would be nice to have this test check for the proper number of <tr> elements before the clear-table.

Also, given that you have the is_ready() method this may not be helpful, but in wildcards I added a wait_for_no_elements method

Marc-André Rivet added 2 commits April 14, 2020 14:32
- add pytest.ini with `testpaths`
testpaths = tests/
addopts = -rsxX -vv
log_format = %(asctime)s | %(levelname)s | %(name)s:%(lineno)d | %(message)s
log_cli_level = ERROR
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as for DCC - accelerates -k test runs
plotly/dash-core-components#740

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃

@Marc-Andre-Rivet Marc-Andre-Rivet merged commit 5f545f1 into dev Apr 20, 2020
@Marc-Andre-Rivet Marc-Andre-Rivet deleted the 384-validate-data-columns branch April 20, 2020 21:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validate data attribute
3 participants