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

Sorting a reordered column (w/ ColReorder) sorts column at original position #1069

Closed
3 tasks done
isthisthat opened this issue Jun 6, 2023 · 13 comments · Fixed by #1096
Closed
3 tasks done

Sorting a reordered column (w/ ColReorder) sorts column at original position #1069

isthisthat opened this issue Jun 6, 2023 · 13 comments · Fixed by #1096

Comments

@isthisthat
Copy link

isthisthat commented Jun 6, 2023

Not sure whether this is a regression since I can see it working fine on the demo page.
Here's an example where I've swapped the position of two columns and ordered by one (gc_rank), which results in the table being ordered by the other (c_rank), which is the original position of the "gc_rank" column. Having the same issue even when I keep all other settings as default and not loading any other extensions. I'm using DT v0.28 in shiny v1.7.4.
image

> xfun::session_info('DT')
R version 4.1.2 (2021-11-01)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 22.04.1 LTS, RStudio 2022.7.2.576

Locale:
  LC_CTYPE=en_GB.UTF-8       LC_NUMERIC=C               LC_TIME=en_GB.UTF-8        LC_COLLATE=en_GB.UTF-8     LC_MONETARY=en_GB.UTF-8   
  LC_MESSAGES=en_GB.UTF-8    LC_PAPER=en_GB.UTF-8       LC_NAME=C                  LC_ADDRESS=C               LC_TELEPHONE=C            
  LC_MEASUREMENT=en_GB.UTF-8 LC_IDENTIFICATION=C       

Package version:
  base64enc_0.1.3   bslib_0.4.2       cachem_1.0.8      cli_3.6.1         crosstalk_1.2.0   digest_0.6.31     DT_0.28           ellipsis_0.3.2   
  evaluate_0.21     fastmap_1.1.1     fontawesome_0.5.1 fs_1.6.2          glue_1.6.2        graphics_4.1.2    grDevices_4.1.2   highr_0.10       
  htmltools_0.5.5   htmlwidgets_1.6.2 jquerylib_0.1.4   jsonlite_1.8.4    knitr_1.42        later_1.3.1       lazyeval_0.2.2    lifecycle_1.0.3  
  magrittr_2.0.3    memoise_2.0.1     methods_4.1.2     mime_0.12         promises_1.2.0.1  R6_2.5.1          rappdirs_0.3.3    Rcpp_1.0.10      
  rlang_1.1.1       rmarkdown_2.21    sass_0.4.6        stats_4.1.2       stringi_1.7.12    stringr_1.5.0     tinytex_0.45      tools_4.1.2      
  utils_4.1.2       vctrs_0.6.2       xfun_0.39         yaml_2.3.7       

By filing an issue to this repo, I promise that

  • I have fully read the issue guide at https://yihui.org/issue/.
  • I have provided the necessary information about my issue.
    • If I'm asking a question, I have already asked it on Stack Overflow or RStudio Community, waited for at least 24 hours, and included a link to my question there.
    • If I'm filing a bug report, I have included a minimal, self-contained, and reproducible example, and have also included xfun::session_info('DT'). I have upgraded all my packages to their latest versions (e.g., R, RStudio, and R packages), and also tried the development version: remotes::install_github('rstudio/DT').
    • If I have posted the same issue elsewhere, I have also mentioned it in this issue.
  • I have learned the Github Markdown syntax, and formatted my issue correctly.

I understand that my issue may be closed if I don't fulfill my promises.

@isthisthat
Copy link
Author

Figured this is another broken extension when server=TRUE.
Minimal example:

renderDT({
  datatable(data.frame(a=c(1,4,2,5,3),b=c(5,4,3,2,1)),
          options = list(colReorder = TRUE),
          extensions = "ColReorder",
  )
}, server=TRUE)

Then reorder column a to move it to the right and sort by it. The table should be sorting by column b.
image

@yihui Can you please check whether this works for you? Any chance of fixing this? Thanks

@yihui
Copy link
Member

yihui commented Oct 18, 2023

In theory, extensions are supposed to work with the server = FALSE mode. The server = TRUE mode is an implementation in DT and not in the upstream DataTables library. Supporting all extensions can require a lot of work. Unfortunately, we don't have enough human resources for this package. Unless your data is huge, I'd recommend using the client-side mode if you must use extensions.

Thanks for your understanding!

@philibe
Copy link

philibe commented Oct 19, 2023

About the maintenance of DT and the xkcd cartoon :

75% of my big R Shiny project is made on DT. All of the datas, except graphs, are displayed by DT in this project to display and research datas computed the last night or interactively in R. I knew that you, @shrektan and @stla are the maintainers. But I didn't know that you, Yihui, are "similar to the supporting pole in the cartoon" nor "[DT] will enter the maintenance-only mode, meaning that I will not add substantial new features to it anymore. " oO.

But I understand the difficulties. Since my beginning in R ecosystem in 2018 I've made lot of tweaks (js, R, Shiny, DT) on my R project, but to be a maintener in a public package is a harder work especially if it is based on an external JS library (DataTables) : to change something will be more and more like play to Mikado Game, because it's becamed a big project and you cannot risk to break existing DT projects, without enough maintainers and time.

The more powerful ability of DT is to automatically paginate and research lot of datas, in the server side, without the R users explicitly write pagination :)

Thanks for you work :)

@isthisthat
Copy link
Author

In theory, extensions are supposed to work with the server = FALSE mode. The server = TRUE mode is an implementation in DT and not in the upstream DataTables library. Supporting all extensions can require a lot of work. Unfortunately, we don't have enough human resources for this package. Unless your data is huge, I'd recommend using the client-side mode if you must use extensions.

Thanks for your understanding!

Thanks @yihui and congratulations on the brilliant work. It has made a huge impact to our work. Completely understand the restrictions. I'm wondering whether Posit might be interested in monetizing it (and therefore putting more resources into it) since it's really one of a kind and unparalleled in functionality. We would definitely be interested in a commercial license if that were the case.

@yihui
Copy link
Member

yihui commented Oct 27, 2023

I spent some time today on investigating this issue. Unfortunately, I find this impossible to fix on my side at the moment. The reason is that the DataTables library doesn't give me any information about the reordered columns, so on the server (R) side, I have no idea which column I should use to sort the data. This problem has to be fixed on the DataTables side. The ColReorder extension has to send the correct column number to the server when columns are reordered. For example, if the first column is moved to the second, and the user sorts the second column, the server should know that the user is actually sorting the first column in the original data instead of the second column. You will have to contact the DataTables support to fix this issue: https://datatables.net/support/index

I'm wondering whether Posit might be interested in monetizing it (and therefore putting more resources into it) since it's really one of a kind and unparalleled in functionality. We would definitely be interested in a commercial license if that were the case.

@isthisthat Thanks for the suggestion! That's definitely a possibility. Money is less of a problem to us. The hard problem is to find a person who can and wants to make this commitment.

to change something will be more and more like play to Mikado Game, because it's become a big project and you cannot risk to break existing DT projects, without enough maintainers and time.

@philibe Exactly. DataTables is complicated :)

Collapse

@isthisthat
Copy link
Author

Thanks @yihui really appreciate the gesture. We'll look into DataTables support

@yihui
Copy link
Member

yihui commented Nov 15, 2023

@isthisthat Good news: I just learned a way to fix this issue and should get it done today. I'll report back later.

@isthisthat
Copy link
Author

@yihui that is literally a Christmas miracle ❤️ Thank you 🙇
Not sure if that way also applies to the search builder extension 🧐 ?

@yihui yihui linked a pull request Nov 15, 2023 that will close this issue
@yihui
Copy link
Member

yihui commented Nov 16, 2023

It took me a whole day to fix this issue, and I'm so glad that it's done now. You can test the development version via

remotes::install_github('rstudio/DT')

Regarding the SearchBuilder extension, it still doesn't work with the server = TRUE mode: #881 (comment)

yihui added a commit that referenced this issue Nov 16, 2023
@isthisthat
Copy link
Author

isthisthat commented Nov 17, 2023

I can confirm that reordering now works with server=TRUE. Thank you so much @yihui 🥳
A minor thing that now happens when reordering and sorting is that any formatStyle originally applied to the column disappears.

@yihui
Copy link
Member

yihui commented Nov 17, 2023

You are welcome! The formatStyle issue may be harder to fix. I'll see.

@yihui
Copy link
Member

yihui commented Nov 21, 2023

I have spent some time on investigating the formatStyle issue. I think it's technically possible to fix it, but it's a little involved. I need a fresh mind to tackle it again, perhaps after the holiday season. If it is important to you, please feel free to remind me in January.

@isthisthat
Copy link
Author

Thanks @yihui will do! Hope you have a relaxing break!!

yihui added a commit that referenced this issue Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants