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

[Bug]: The filter panel feature does not work well when data/vars have special characters #570

Open
3 tasks done
vedhav opened this issue Mar 11, 2024 · 7 comments
Open
3 tasks done
Labels
bug Something isn't working core

Comments

@vedhav
Copy link
Contributor

vedhav commented Mar 11, 2024

What happened?

The namespace creation function for the data and column names can be more robust. Right now the special characters are stripped from the name space which might not be ideal.

library(teal)

app <- init(
  within(teal_data(), {
    iris <- iris
    iris[["Species!"]] <- iris$Species
    iris[["Species!@#$%^&*_-+={[()]}:;,<>./?"]] <- iris$Species
  }),
  modules(
    example_module("Module 1"),
    example_module("Module 2"),
    modules(
      label = "Nested modules",
      example_module("Module 3"),
      example_module("Module 4"),
      modules(
        label = "Sub nested modules",
        example_module("Module 5"),
        example_module("Module 6")
      )
    )
  ),
  filter = teal_slices(
    teal_slice(dataname = "iris", varname = "Species", multiple = FALSE),
    teal_slice(dataname = "iris", varname = "Species!", multiple = TRUE),
    teal_slice(dataname = "iris", varname = "Species!@#$%^&*_-+={[()]}:;,<>./?", multiple = TRUE),
    teal_slice(dataname = "iris", varname = "Sepal.Length", multiple = TRUE)
  )
)

shinyApp(app$ui, app$server)

sessionInfo()

No response

Relevant log output

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct.

Contribution Guidelines

  • I agree to follow this project's Contribution Guidelines.

Security Policy

  • I agree to follow this project's Security Policy.
@vedhav vedhav added bug Something isn't working core labels Mar 11, 2024
@vedhav vedhav changed the title [Bug]: The filter panel feature do not work well data has columns with special characters [Bug]: The filter panel feature do not work well when data/vars have special characters Mar 11, 2024
@m7pr m7pr changed the title [Bug]: The filter panel feature do not work well when data/vars have special characters [Bug]: The filter panel feature does not work well when data/vars have special characters Mar 11, 2024
@chlebowa
Copy link
Contributor

What are you trying to accomplish here? "Species!@#$%^&*_-+={[()]}:;,<>./?" is not a syntactically valid column name.
Also, the issue description is not clear. What is the relationship between a column name and a namespace? How would that string end up in a namespace function?

@vedhav
Copy link
Contributor Author

vedhav commented Mar 11, 2024

Although there is no practical use of having this column. I created this column to showcase the underlying problem. It is possible to pass data with these columns. We have to handle it. We might simply show a warning when such columns are passed, but we need some way to address this.

names(iris)
[1] "Sepal.Length"                      "Sepal.Width"                       "Petal.Length"
[4] "Petal.Width"                       "Species"                           "Species!"
[7] "Species!@#$%^&*_-+={[()]}:;,<>./?"

If you inspect the namespace you'll notice that the namespaces are as follows:

Column Name Name-space
Species Species
Species! Species_
Species!!! Species_
Species!@#$%^&*_-+={[()]}:;,<>./? Species_

@chlebowa
Copy link
Contributor

We have to handle it.

No we don't. These are not syntactically valid column names. They wouldn't fly in an interactive session and I see no reason why they should be tolerated in teal.

If you inspect the namespace you'll notice that the namespaces are as follows:

What about Sepal.Width and the others?

@vedhav
Copy link
Contributor Author

vedhav commented Mar 11, 2024

What about Sepal.Width and the others?

Sepal.Width becomes Sepal_Width. Evidently the special character combination is converted to _
So Sepal..Width also becomes Sepal_Width

@chlebowa
Copy link
Contributor

So full stops are not JS friendly, which is why they are converted to underscores. I don't see why duplicates aren't maintained.
There is an improvement to be made here but I don't think we should strive to handle ridiculous stuff.

I think a simple make.names(x, unique = TRUE) followed by gsub(".", "_", x, fixed = TRUE) is quite enough.

@pawelru
Copy link
Contributor

pawelru commented Mar 11, 2024

In R it is possible to put garbage names in quotes, e.g. things like: df$`-a` <- 1. This will accept anything.

I think that the underlying issue is the fact that this names most likely ends up being a HTML ID string. Just because of that there might be column names validation down the way from user inputs to actuall app HTML creation.

I think we saw issues about that already but can't find these now.

The ultimate solution would be to replace creating IDs from column names with some hashing functionality that will be (i) unique and (ii) valid string accepted as an ID. I think that any gsub based solution can be challenged once you know the pattern and replacement :)
Note that this would most likely break many shinytest2 tests because it's a common practice to relay on ID of gui elements.

The other aspect is how likely it is. Here I agree with @chlebowa that this probably needs improvement but IMHO it's quite rare. You should not do df$`<incorrect name>` type of stuff as most likely it's not only teal that would complain

@kpagacz
Copy link
Contributor

kpagacz commented Mar 19, 2024

I don't think anything would complain about df$<incorrect name> as long as it doesn't end up being an id of an HTML element.

The fact of the matter is, in R, anything inside `` is a valid identifier according to the language specification.

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

No branches or pull requests

4 participants