Provide support for no_update in Dash for R

[rpkyle]

This PR proposes to introduce a function entitled dashNoUpdate() which functions analogously to dash.no_update in Python, although the implementation differs slightly.

As with its counterpart, the function permits developers to selectively prevent one or more outputs from updating the layout. Currently, dashNoUpdate() only supports single-output callbacks, but should be easily extended to the multiple outputs scenario when that feature is available in R.

Additionally, the response$body returned is an empty character string, with a response$status of 204:

dashR/R/dash.R

Lines 294 to 299 in b23c248

	# inspect the output_value to determine whether any outputs have no_update
	# objects within them; these should not be updated
	if (class(output_value) == "no_update") {
	response$body <- character(1) # return empty string
	response$status <- 204L
	}

Closes #25.

@Marc-Andre-Rivet

[alexcjohnson]

Looks great!

Now that we have a test framework, let's get in the habit of testing new features as they come in here. You should be able to essentially just port this one test, but simplify it - take out the Value stuff, and you only have one variant to test as opposed to that test covering both no_update and PreventUpdate:

https://github.com/plotly/dash/blob/master/tests/integration/test_integration.py#L120-L178

[rpkyle]

@alexcjohnson I identified a minor issue today while reviewing app PRs with the new code, the following warning appears periodically for some applications:

warning: the condition has length > 1 and only the first element will be used from if (class(output_value) == "no_update") {

I'll resolve this and comment in this PR when the commit is made, in addition to adding in the test we discussed.

[rpkyle]

Looks great!

Now that we have a test framework, let's get in the habit of testing new features as they come in here. You should be able to essentially just port this one test, but simplify it - take out the Value stuff, and you only have one variant to test as opposed to that test covering both no_update and PreventUpdate:

https://github.com/plotly/dash/blob/master/tests/integration/test_integration.py#L120-L178

fixed in d1f384f

@byronz

[byronz]

fixed in d1f384f

@rpkyle I would like a change in the test with the new proposed API in plotly/dash#859

[alexcjohnson]

The current test failure - where the value hasn't updated between the .click() call and the assert - is why I used .wait_for_text_to_equal in the dash-py test. In general there should be some sort of "wait for" after any callback, then additional tests can be bare assert statements.

In this case the dashNoUpdate() calls mean we'd be waiting for the text to be what it already was - so in fact if the test is fast enough we're not really testing anything. That's why the dash-py test uses the multiprocessing Value in until(lambda: callback1_count.value == 4,...). Since we can't do that here, I'd suggest just waiting a little while (time.sleep(1)?) before testing the text when it's intended not to change.

[byronz]

The current test failure - where the value hasn't updated between the .click() call and the assert - is why I used .wait_for_text_to_equal in the dash-py test. In general there should be some sort of "wait for" after any callback, then additional tests can be bare assert statements.

In this case the dashNoUpdate() calls mean we'd be waiting for the text to be what it already was - so in fact if the test is fast enough we're not really testing anything. That's why the dash-py test uses the multiprocessing Value in until(lambda: callback1_count.value == 4,...). Since we can't do that here, I'd suggest just waiting a little while (time.sleep(1)?) before testing the text when it's intended not to change.

this represents a common scenario where you would like to check something after an action and would like to wait a little bit so the assertion was not made too early to test nothing.

instead of the sleep then assert (our current way), I'm proposing to have sth similar to wait.until or wait.until_not: it does more assertions during that sleep period with a poll=0.1.

wait.check(lambda: out.text.endswith('#FF0000'), duration=0.5)

[alexcjohnson]

@byronz not sure I quite follow you re: wait.check but let's have this discussion in an issue over in the dash repo. As in my previous comment I'd like to get this merged using the existing dash.testing API, then we can come back and clean it up in a future PR.

[byronz]

@byronz not sure I quite follow you re: wait.check but let's have this discussion in an issue over in the dash repo. As in my previous comment I'd like to get this merged using the existing dash.testing API, then we can come back and clean it up in a future PR.

@alexcjohnson I'm not talking about change this right now in this PR, but something we might need as test API like wait.check, what it does is consistently check the condition for a specified amount of time.

[rpkyle]

@alexcjohnson I identified a minor issue today while reviewing app PRs with the new code, the following warning appears periodically for some applications:

warning: the condition has length > 1 and only the first element will be used from if (class(output_value) == "no_update") {

I'll resolve this and comment in this PR when the commit is made, in addition to adding in the test we discussed.

fixed in ae7b1b0

[rpkyle]

The current test failure - where the value hasn't updated between the .click() call and the assert - is why I used .wait_for_text_to_equal in the dash-py test. In general there should be some sort of "wait for" after any callback, then additional tests can be bare assert statements.

In this case the dashNoUpdate() calls mean we'd be waiting for the text to be what it already was - so in fact if the test is fast enough we're not really testing anything. That's why the dash-py test uses the multiprocessing Value in until(lambda: callback1_count.value == 4,...). Since we can't do that here, I'd suggest just waiting a little while (time.sleep(1)?) before testing the text when it's intended not to change.

fixed in 8f6dafd

[alexcjohnson]

Not to beat a dead horse, but since this is one of the first tests I'd like to make sure we get the best pattern for all the other tests to build on.

time.sleep should be a last resort. In this test you only need it when the text is expected to be the same after the action as it was before - ie just the checks after ...[3].click(). But in all the other cases you can do

dashr.wait_for_text_to_equal(
    "#message-box",
    "The hexadecimal representation of your last chosen color is #FF0000"
)

Which will return as soon as the correct value is found on the page, saving us most of that 1 sec delay each time.

[rpkyle]

fixed in be9d387

[alexcjohnson]

💃 Alright, one more feature toward parity!