Skip to content

Make data viewer openable from the variables window while debugging#14406

Merged
joyceerhl merged 24 commits intomainfrom
tensor-variable-context-menu
Oct 19, 2020
Merged

Make data viewer openable from the variables window while debugging#14406
joyceerhl merged 24 commits intomainfrom
tensor-variable-context-menu

Conversation

@joyceerhl
Copy link
Copy Markdown

Provide users with a way to open the data viewer for data-viewable types, i.e.:

  • 'DataFrame'
  • 'list'
  • 'dict'
  • 'ndarray'
  • 'Series'

In an interactive window debug session:
dataframes

In a regular Python debugging session:
debugsinglefile

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR).
  • Title summarizes what is changing.
  • Has a news entry file (remember to thank yourself!).
  • Appropriate comments and documentation strings in the code.
  • Has sufficient logging.
  • Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated.
  • Test plan is updated as appropriate.
  • package-lock.json has been regenerated by running npm install (if dependencies have changed).
  • The wiki is updated with any design decisions/details.

await this.evaluate(DataFrameLoading.VariableInfoImport);
this.importedIntoKernel.add(key);
}
await this.evaluate(DataFrameLoading.DataFrameSysImport);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Probably don't want to do this more than once per debug session. It's rather slow.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@rchiodo would it suffice to use the activeDebugSession's ID instead of the stringified notebook identity like so:

if (this.debugService.activeDebugSession && !this.importedIntoKernel.has(this.debugService.activeDebugSession.id)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not sure debug session ids are unique (might get reused), but if you cleared the cache for that id on shutdown, that should work.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VSC docstring says it's unique:

    /**
     * A debug session.
     */
    export interface DebugSession {
        /**
         * The unique ID of this debug session.
         */
        readonly id: string;

Do we have a reason to think the IDs might not be unique?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unique for the list of active sessions running at the same time. May not be unique if you stop and start debugging.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I guess I'd want to look to see how it's generated. If it's a guid, then it's probably unique for all time. If it's a counter, then it's unique enough to not matter.

or we can just remove the flag of 'set' in debugger when a session stops.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It's a GUID, btw

await this.dataViewerFactory.create(jupyterVariableDataProvider, title);
}
} catch (e) {
this.appShell.showErrorMessage(e.toString());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd stick a traceError here as well so that it's in the log.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 15, 2020

Codecov Report

Merging #14406 into main will decrease coverage by 0.23%.
The diff coverage is 17.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #14406      +/-   ##
==========================================
- Coverage   59.40%   59.17%   -0.24%     
==========================================
  Files         716      720       +4     
  Lines       40000    40211     +211     
  Branches     5793     5829      +36     
==========================================
+ Hits        23763    23794      +31     
- Misses      14975    15152     +177     
- Partials     1262     1265       +3     
Impacted Files Coverage Δ
...cience/data-viewing/jupyterVariableDataProvider.ts 14.00% <0.00%> (ø)
...data-viewing/jupyterVariableDataProviderFactory.ts 50.00% <0.00%> (ø)
...active-common/intellisense/intellisenseProvider.ts 33.24% <0.00%> (ø)
...ience/interactive-common/interactiveWindowTypes.ts 100.00% <ø> (ø)
src/client/datascience/jupyter/jupyterVariables.ts 27.77% <0.00%> (ø)
src/client/datascience/types.ts 100.00% <ø> (ø)
src/client/telemetry/index.ts 80.41% <ø> (ø)
...rc/client/datascience/jupyter/debuggerVariables.ts 8.21% <2.50%> (ø)
.../datascience/interactive-common/interactiveBase.ts 5.93% <16.66%> (+0.15%) ⬆️
...atascience/interactive-common/dataViewerChecker.ts 20.83% <20.83%> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa1b0b2...e93fcf5. Read the comment docs.

) {
// This should be the top frame. We need to use this to compute the value of a variable
this.updateStackFrame(message as DebugProtocol.StackTraceResponse);
} else if (message.type === 'event' && message.event === 'terminated') {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Right here is where you'd clean out the appropriate spot in the importedIntoKernel map

@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@joyceerhl joyceerhl merged commit 20349f2 into main Oct 19, 2020
@joyceerhl joyceerhl deleted the tensor-variable-context-menu branch October 19, 2020 16:18
rchiodo pushed a commit that referenced this pull request Oct 19, 2020
* Ensure debug adapter request/response sequence numbers tally (#14336)

* Make data viewer openable from the variables window while debugging (#14406)
luabud pushed a commit to luabud/vscode-python that referenced this pull request Oct 26, 2020
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 21, 2025

Codecov Report

❌ Patch coverage is 17.54386% with 94 lines in your changes missing coverage. Please review.
✅ Project coverage is 59%. Comparing base (aa1b0b2) to head (e93fcf5).
⚠️ Report is 2711 commits behind head on main.

Files with missing lines Patch % Lines
...rc/client/datascience/jupyter/debuggerVariables.ts 2% 39 Missing ⚠️
...atascience/interactive-common/dataViewerChecker.ts 20% 19 Missing ⚠️
src/client/datascience/commands/commandRegistry.ts 34% 15 Missing ⚠️
...cience/data-viewing/jupyterVariableDataProvider.ts 0% 7 Missing ⚠️
src/client/datascience/jupyter/jupyterVariables.ts 0% 7 Missing ⚠️
.../datascience/interactive-common/interactiveBase.ts 16% 5 Missing ⚠️
...data-viewing/jupyterVariableDataProviderFactory.ts 0% 1 Missing ⚠️
...active-common/intellisense/intellisenseProvider.ts 0% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #14406    +/-   ##
=======================================
- Coverage     59%      59%    -1%     
=======================================
  Files        716      720     +4     
  Lines      40000    40211   +211     
  Branches    5793     5829    +36     
=======================================
+ Hits       23763    23802    +39     
- Misses     14975    15147   +172     
  Partials    1262     1262            
Files with missing lines Coverage Δ
src/client/datascience/constants.ts 99% <100%> (+<1%) ⬆️
...ience/interactive-common/interactiveWindowTypes.ts 100% <ø> (ø)
src/client/datascience/types.ts 100% <ø> (ø)
src/client/telemetry/index.ts 80% <ø> (ø)
...data-viewing/jupyterVariableDataProviderFactory.ts 50% <0%> (ø)
...active-common/intellisense/intellisenseProvider.ts 33% <0%> (ø)
.../datascience/interactive-common/interactiveBase.ts 5% <16%> (+<1%) ⬆️
...cience/data-viewing/jupyterVariableDataProvider.ts 14% <0%> (ø)
src/client/datascience/jupyter/jupyterVariables.ts 27% <0%> (ø)
src/client/datascience/commands/commandRegistry.ts 34% <34%> (+<1%) ⬆️
... and 2 more

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

5 participants