Skip to content
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

Filtering action crashes the app #37

Closed
tom-gu opened this issue Jul 18, 2020 · 5 comments
Closed

Filtering action crashes the app #37

tom-gu opened this issue Jul 18, 2020 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@tom-gu
Copy link
Member

tom-gu commented Jul 18, 2020

image.png

Console error:
image.png

Filtering functions in bdchecks:
https://github.com/bd-R/bdchecks/blob/dev/R/filter.R

Brunches:

bdchecks>>dev | bdchecks.app>>dev

@tom-gu tom-gu added the bug Something isn't working label Jul 18, 2020
@tom-gu tom-gu added this to the rOpenSci Submission milestone Jul 18, 2020
@rahulchauhan049
Copy link
Member

After spending a few hours, I believe the issue occurs because bdchecks::perform_dc (https://github.com/bd-R/bdchecks.app/blob/master/R/mod_perform_checks.R#L82) return error when data contain fields with time stamp like dateIdentified or eventDate, etc.

Error this function return is: Error in as.POSIXlt.character(x, tz, ...) :
character string is not in a standard unambiguous format

@tom-gu
Copy link
Member Author

tom-gu commented Aug 2, 2020

Hi Rahul,

The date error is a known one. I hoped we've already purged it, but it still lurks around. @Martis6 will take care of it (bd-R/bdchecks#27).

However, the filtering function gives an error even when performing bug-free checks.

@rahulchauhan049 or @thiloshon can you please exactly describe us the input coming from the Shiny app (i.e., cell_selected from https://github.com/bd-R/bdchecks/blob/dev/R/filter.R). That way @Martis6 can check it from his end and even build a unit testing.

@tom-gu
Copy link
Member Author

tom-gu commented Aug 6, 2020

@Martis6
The cell_selected input (i.e., input$summaryTable_cells_selected created via the shiny app) is a simple matrix.
The 1st col of the matrix denoting row number (notice that a row/data-check without a selection will not be shown), and the 2nd col denoting the col that the user chose to filter by:

  • 2 is for passed
  • 3 is for failed
  • 4 is for missing

Example:

For the following table selection:
image

This is the matrix:
image

This matrix is also attached (RData file):
summaryTable_cells_selected.zip

@tom-gu
Copy link
Member Author

tom-gu commented Aug 11, 2020

@Martis6 @thiloshon @rahulchauhan049
The problem is simply a missing data argument (😭) in:

clean_data <<- bdchecks:::dc_filter(check_result, summary_filter)

We can fix the issue by adding:
clean_data <<- bdchecks:::dc_filter(user_data, check_result, summary_filter)
Or
clean_data <<- bdchecks:::dc_filter(bdutilities::return_core(user_data), check_result, summary_filter)

Which one is better, and why?

Lesson: To improve our function's defensive programming 🏆 bd-R/bdchecks#28

@thiloshon
Copy link
Collaborator

return_core is better as we can't predict when user_data is reactive or not. return_core always returns the core object if a reactive value is passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants