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

fallback to searching for object "anywhere" #13327

Merged
merged 4 commits into from
Jul 11, 2023

Conversation

sharon-wang
Copy link
Member

@sharon-wang sharon-wang commented Jul 6, 2023

Intent

Addresses:

Approach

The existing code passes an env + obj combination that will not result in the object being found.

In this PR:

  • we already fallback to looking for the data "anywhere" in .rs.getHelpDataFrame
    • when we do this, don't include the env when constructing the gridviewer uri, because the data was found "anywhere"
  • in DataViewer.cpp, if we can't find the data frame using existing methods, try to find it "anywhere" before giving up and erroring

Using this repro:

test_df <- data.frame(col1 = rep(1, 3),
                      col2 = rep(2, 3),
                      col3 = rep(3, 3))

test_ls <- list(a = test_df,
                b = test_df)

# type `test_ls$` to trigger help preview
current this PR
env uri param test_ls empty
obj uri param a test_ls$a
result can't find a in test_ls (test_ls is not an environment) finds test_ls$a "anywhere"

Automated Tests

none

QA Notes

In addition to the repro steps from #13291, I tested an additional nesting layer:

test_df <- data.frame(col1 = rep(1, 3),
                      col2 = rep(2, 3),
                      col3 = rep(3, 3))

test_ls <- list(a = test_df,
                b = test_df)

test_nest <- list(x = test_ls, y = test_ls, z = test_df)

# trigger help preview for the following:
# test_ls$
# test_nest$
# test_nest$x$a
# test_nest$z

Documentation

none

Checklist

  • If this PR adds a new feature, or fixes a bug in a previously released version, it includes an entry in NEWS.md
    - [ ] If this PR adds or changes UI, the updated UI meets accessibility standards
  • A reviewer is assigned to this PR (if unsure who to assign, check Area Owners list)
  • This PR passes all local unit tests

@sharon-wang sharon-wang marked this pull request as ready for review July 10, 2023 19:07
Copy link
Contributor

@kevinushey kevinushey left a comment

Choose a reason for hiding this comment

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

LGTM!

@kevinushey kevinushey merged commit fc483ad into main Jul 11, 2023
1 check passed
@kevinushey kevinushey deleted the bugfix/data-preview-for-list-of-dfs branch July 11, 2023 04:28
sharon-wang added a commit that referenced this pull request Jul 14, 2023
* fallback to searching for object "anywhere"

* update NEWS.md
sharon-wang added a commit that referenced this pull request Jul 14, 2023
* fallback to searching for object "anywhere"

* update NEWS.md
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.

None yet

2 participants