Repository navigation
Update gather functionality to support 0.4.1 of python-program-analysis - #8219
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8219 +/- ##
==========================================
+ Coverage 59.23% 59.35% +0.11%
==========================================
Files 508 509 +1
Lines 23236 23267 +31
Branches 3750 3758 +8
==========================================
+ Hits 13765 13809 +44
+ Misses 8577 8559 -18
- Partials 894 899 +5
Continue to review full report at Codecov.
|
This was changed so it should have failed? Refers to: src/test/datascience/gather/gather.unit.test.ts:147 in af781c1. [](commit_id = af781c1, deletion_comment = False) |
| "description": "Enable code gather for executed cells. For a gathered cell, that cell and only the code it depends on will be exported to a new notebook.", | ||
| "scope": "resource" | ||
| }, | ||
| "python.dataScience.gatherToScript": { |
There was a problem hiding this comment.
This seems weird. Why not just always open a notebook if coming from a notebook and always open a file if coming from a file? For interactive window users, they'd never find this command and would be put off by us opening a notebook
| } | ||
|
|
||
| if (this.notebook) { | ||
| const uri: Uri = await this.getNotebookIdentity(); |
There was a problem hiding this comment.
await this.getNotebookIdentity [](start = 29, length = 30)
This second await is redundant. You could just cache the value from the first call.
| return Promise.resolve(); | ||
| } | ||
|
|
||
| public gatherCode(_c: ICell): string | undefined { |
There was a problem hiding this comment.
You can remove these now too.
| traceInfo('Gathering tools have been activated'); | ||
| } | ||
| public logExecution(vscCell: IVscCell): void { | ||
| const gatherCell = convertVscToGatherCell(vscCell); |
There was a problem hiding this comment.
So I'm a bit nervous that we lost our cloneDeep here. Technically convert should not change the vscCell, but I'm worried someone else might tweak it. Was it a perf issue? #ByDesign
There was a problem hiding this comment.
Talked with Rich, we'll take care of this before calling postExecute, so disregard this.
In reply to: 342290179 [](ancestors = 342290179)
|
Thanks for the help Rich! |
Because that function is called A LOT. Every time we update code lenses. I wanted to preserver the O(n) of it if possible. And in case there were bugs, this will only affect gather then. |
6 participants
For #8202
Has a news entry file (remember to thank yourself!)Has sufficient logging.Has telemetry for enhancements.Unit tests & system/integration tests are added/updatedTest plan is updated as appropriatepackage-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed)