Skip to content

Conversation

@joyceerhl
Copy link

image

  • 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.

@joyceerhl joyceerhl added the no-changelog No news entry required label Oct 3, 2020
@codecov-commenter
Copy link

codecov-commenter commented Oct 3, 2020

Codecov Report

Merging #14244 into main will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #14244      +/-   ##
==========================================
- Coverage   59.96%   59.96%   -0.01%     
==========================================
  Files         706      706              
  Lines       39180    39180              
  Branches     5681     5681              
==========================================
- Hits        23494    23493       -1     
- Misses      14449    14450       +1     
  Partials     1237     1237              
Impacted Files Coverage Δ
src/client/common/utils/platform.ts 68.00% <0.00%> (-4.00%) ⬇️

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 ef44f29...f70bed8. Read the comment docs.

import vscodeDataFrameHelpers as _VSCODE_dataFrameHelpers


def maybeParseTensorShape(var, result):
Copy link
Member

Choose a reason for hiding this comment

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

With all of our variable fetching code we prefixed with VSCODE to avoid conflict with something the user has define. I would think that we would need this here are well.

def maybeParseTensorShape(var, result):
try:
vartype = type(var)
if (hasattr(vartype, "__name__")) and vartype.__name__ == "Tensor":

Choose a reason for hiding this comment

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

Should we add a simple test for this?

Copy link
Author

Choose a reason for hiding this comment

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

@DonJayamanne do you mean a Python test or a Typescript test? And would that involve installing PyTorch on the build machines?

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 6, 2020

Kudos, SonarCloud Quality Gate passed!

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

No Coverage information No Coverage information
No Duplication information No Duplication information

@DonJayamanne DonJayamanne merged commit a24d64a into microsoft:main Oct 9, 2020
luabud pushed a commit to luabud/vscode-python that referenced this pull request Oct 26, 2020
DonJayamanne pushed a commit that referenced this pull request Oct 30, 2020
…-jupyter (#194)

* Fix sample notebook to state that you can double click to edit a cell (#14238)

* Disable surveys if running in codespaces (#14332)

* Disable surveys if running in codespaces

* Disable gather survey in codespaces too

* For gather, show a description without the survey

* Fixes for IW tests where UIKind is not defined

* Fix all the tests where UIKind is not defined

* Appease hygiene

* Disable mailing list if running in Codespaces (#14347)

* Show tensor dimensions in variable explorer (#14244)

* Fix markdown cell marker formatting for export to Python (#14360)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog No news entry required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants