Conversation
| ########################################################################### | ||
| # 2. Filters the data and/or removes missing values | ||
| ########################################################################### | ||
| if (isScatter(chart.type) && !is.null(input.data.raw) && containsQTable(input.data.raw)) |
There was a problem hiding this comment.
Previously we distinguished whether filters should be applied to the X/Y/Z coordinates dropbox by checking whether it was a raw variable or a summary table based on the attributes. But the dropbox controls now accept calculations, and they are often missing attributes if the use is only extracting a column etc, so we just check lengths
…o not remove anything
| "To apply filters you need to instead filter the source data that is being visualized.") | ||
| "To apply filters you need to instead filter the source data that is being visualized.") | ||
| tb.desc <- attr(x, "basedescription") | ||
| if (is.null(tb.desc) || tb.desc$FilteredProportion == 0) |
There was a problem hiding this comment.
The extra check on tb.desc$FilteredProportion == 0 is unnecessary because of the check in the else if block. It is also causing the warning to show when it shouldn't. For example when Awareness - Coca-Cola is set as a filter, because of 100% of respondents were aware of this brand, the filtered proportion was 0.
There was a problem hiding this comment.
For the example given in RS-20371, most of the warnings are caused by https://github.com/Displayr/q/pull/23862/changes#diff-6f96a7535848bb56e74401d118d945905d6eb60a726c1904399d5953c4f42a89L4857 but this is the cause of the warning in RS-18209.
R/preparedata.R
Outdated
| if (isScatter(chart.type) && !is.null(input.data.raw) && containsQTable(input.data.raw)) | ||
| # Ignore filter if the scatterplot input is not raw variables. | ||
| # The input can be a Q summary table or some other type of calculation | ||
| use_filter <- length(subset) > 1 && NROW(subset) == NROW(data) |
There was a problem hiding this comment.
It should be use.filter instead
R/preparedata.R
Outdated
| # Ignore filter if the scatterplot input is not raw variables. | ||
| # The input can be a Q summary table or some other type of calculation | ||
| use_filter <- length(subset) > 1 && NROW(subset) == NROW(data) | ||
| if (!use_filter && isScatter(chart.type)) |
There was a problem hiding this comment.
It seems strange that we only need to do this for scatter. What happens with the other charts when use_filter is FALSE?
There was a problem hiding this comment.
I think we only need to ignore filters for scatter because its the only chart where the dropbox controls for the input.data.raw can also accept non-variables
|
|
||
| processPastedData <- function(input.data.pasted, warn, date.format, subset, weights) | ||
| { | ||
| if (length(subset) > 1) |
There was a problem hiding this comment.
The example in the bug appears to be for input data, not pasted data. Do you need to make the same change to processInputData?
If thats the case, will we ever show a warning about filters not being applied? I guess this could be a solution but it means we no longer warn when we should. But Sam made the point about applying a fix to the q repo https://numbers.atlassian.net/browse/RS-20371?focusedCommentId=170229. I imagine we would disable applying page filters to all R calcs that do not have a raw data input?
There was a problem hiding this comment.
I've restored the warning for pasted data. I initially did think about just removing all the warnings, but after some more thinking I think we should keep them where possible.
The fix in the q repo works by not applying the filter on the R viz, only on the nested table, but this is causing some inconsistencies so I am going to revert that: https://github.com/Displayr/q/pull/23862. We also don't really want to add warnings via q because its a bit hard to figure out when the warnings can be turned off.
|
I'm trying to figure out what the effect of these changes are. Based on my understanding: RS-20850 Labelled Scatter Plot returns error when filter is appliedhttps://numbers.atlassian.net/browse/RS-20850
RS-20371 Visualizations incorrectly show filter/weight warnings when filters are applied at folder or page levelhttps://numbers.atlassian.net/browse/RS-20371
Is that correct? |
|
Yes, I think most of the warnings in the document in RS-20371 will be fixed by https://github.com/Displayr/q/pull/23862/changes. But some of the warnings in https://numbers.atlassian.net/browse/RS-18209 will be fixed by removing the check in PrepareData for |
Are you referring to this: |
How does this PR fix the old issue that https://github.com/Displayr/q/pull/20846 was meant to fix? |
|
This PR (particularly https://github.com/Displayr/flipChart/pull/86/changes#diff-1fa145fd3255b55efc51409cb6bde614753d16b220e65fdbea73e7ab3822d76cL1061) stops some incorrect warnings about Filters being ignored from being shown when the referenced/nested table does in fact have the appropriate filter applied. |
Does it fix the original example in https://numbers.atlassian.net/browse/RS-19614 |
2 participants
There have been several bug reports about warnings on filters popping up unnecessarily, but there are cases when it is appropriate to show warnings, so I've tried to preserve the warnings but just fixing the conditions when they are shown.
For example, when a R viz references a table on a different page, and page filters are applied on the viz, but not on the referenced table, then a warning should be shown.