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

feat: Rework view() to better work with RStudio and Positron #1603

Merged
merged 3 commits into from
Sep 30, 2024

Conversation

DavisVaughan
Copy link
Member

@DavisVaughan DavisVaughan commented Sep 30, 2024

Closes #1551 (while I was here, I may as well...)

In RStudio and Positron, when View(foo) is called, we check if foo exists as a symbol bound in the parent environment. If it does, we can "track" the lifetime of that object and auto update the data viewer when the user makes changes to it. This doesn't work with view()

Screen.Recording.2024-09-30.at.10.56.25.AM.mov

This is because in view() we were actually inlining the whole data frame object, rather than the expression the user typed in. So RStudio and Positron can't find anything to "track". This was also causing issues in Positron too actually (the inlined data frame could overwhelm us) posit-dev/positron#4702.

@lionel- and I thought about this a bit and came up with an approach where we capture the user input to view() and use that to reconstruct an equivalent call to View() that we call in the parent environment. This mimics the idea of the user calling View(foo) directly in the global environment, so that RStudio/Positron can "track" foo correctly.

  • In the case that x isn't a data frame, we don't do any of this. We "make" x into a data frame ourselves, so RStudio and Positron won't have anything to "track" in these cases so we don't do anything special
  • In the case that x is a data frame but comes from something like view(cbind(foo, foo)), we do the fancy thing to recall this as View() in the parent frame, which works fine, but of course RStudio and Positron still won't have anything to track because this is an ephemeral object. This is expected.

It works like this now:

Screen.Recording.2024-09-30.at.10.54.40.AM.mov
Screen.Recording.2024-09-30.at.10.55.18.AM.mov

Side note, how do you add news bullets in tibble?

@DavisVaughan DavisVaughan requested a review from krlmlr September 30, 2024 17:42
@krlmlr krlmlr changed the title Rework view() to better work with RStudio and Positron feat: Rework view() to better work with RStudio and Positron Sep 30, 2024
Copy link
Member

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, great!

NEWS bullets are added by CI/CD from the PR title once merged, don't worry about it.

R/view.R Outdated Show resolved Hide resolved
R/view.R Outdated Show resolved Hide resolved
R/view.R Outdated Show resolved Hide resolved
@krlmlr krlmlr merged commit acbf6cc into tidyverse:main Sep 30, 2024
@krlmlr
Copy link
Member

krlmlr commented Sep 30, 2024

Thanks! Happy to release as needed.

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 this pull request may close these issues.

Tweak the view function
2 participants