Python3 Selection Under Debug Modes #10629
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
QilongTang
commented
May 5, 2020
| // Example: | ||
| // AddDebugMode("TestDebugMode", "Enabe/disable TestDebugMode."); | ||
| // Register app wide new debug modes here. | ||
| AddDebugMode("Python3DebugMode", "Enabe/disable Python3DebugMode."); |
Contributor
Author
There was a problem hiding this comment.
@pinzart I have registered a new Debug mode for our Python3 feature and added a debug.config file in Dynamo root. Curious how to reference it?
Contributor
There was a problem hiding this comment.
You can use if (DebugModes.Enabled("Python3DebugMode") == true) to check if your debug mode is enabled or not.
As a side note...I put the DebugModes class in DynamoCore...in the hopes that all other Projects in Dynamo can include the Dynamo.Configuration namespace ...in order to have access to DebugModes.
Contributor
There was a problem hiding this comment.
as for the Description section of the DebugMode...It only helps to figure out what the feature is, behind the debug mode. This is only visible in the DebugModes UI.
You can leave out the fact that it is a debug mode ....Like Enable/Disable Python 3 Engine....or something like that..
Anyway...it's a description...you can put anything you want theere...
Contributor
Author
There was a problem hiding this comment.
@pinzart Adding the name space may not help since I am using the reference in a different dll here, I had to add internally visible to key word. See my changes
Contributor
|
@QilongTang The DebugModes class is basically a static class... |
Contributor
Author
|
@DynamoDS/dynamo @pinzart Did some clean up from my point of view, PTAL |
pinzart
approved these changes
May 5, 2020
Contributor
pinzart
left a comment
There was a problem hiding this comment.
Thanks for the code changes @QilongTang
LGTM
mmisol
approved these changes
May 5, 2020
Collaborator
mmisol
left a comment
There was a problem hiding this comment.
Looks good. Just a comment regarding the TODO but not related to the PR itself.
| if (Engine == PythonEngineVersion.CPython3) | ||
| { | ||
| // TODO: Uncomment this and replace the following code when merging Python3 changes in master | ||
| // pythonEvaluatorMethod = DSCPython.CPythonEvaluator.EvaluatePythonScript; |
Collaborator
There was a problem hiding this comment.
It would be nice to add this in the AC of whatever task will merge the Python3 branch into master. Otherwise we may forget to do this.
Contributor
|
@QilongTang you should be able to add tests that Enable the Python3 DebugMode at the beginning of the test...(then disable at the end-- so it does not interfere with the rest of the tests) |
mjkkirschner
reviewed
May 5, 2020
pinzart
reviewed
May 6, 2020
pinzart
reviewed
May 6, 2020
reddyashish
added a commit
that referenced
this pull request
May 14, 2020
* add namespace conflict tests for short name replacer and node to code (#10611) * Register custom node before package load reset (#10591) This fixes a problem where existing custom nodes in the home workspace became unresolved after a package that contained binaries was loaded. The cause of the problem was that the compiled function was not available at the time of the execution after a VM reset. Now the data is registered on package load, by queueing it in pendingCustomNodeSyncData. This results in a CompileCustomNodeAsyncTask being scheduled before the update of the home workspace graph takes place. * Increase coverage of the Core folder (#10609) * Add a test for the CrashPromptArgs class * Update DynamoCoreTests.csproj * Added NotificationObject tests * Updated the test, NotificationObject coverage at 100% * Changed some access modifiers * Added coverage for Updates/BinaryVersion * Added comments and fixed names * Revert "Added comments and fixed names" This reverts commit 42cd024. * Revert "Added coverage for Updates/BinaryVersion" This reverts commit 679deca. * Increased coverage in CrashPromptArgs * Added some Core coverage * DYN-2560 - Increase the code coverage: DynamoCore/Models Folder First Part (#10612) * DYN-2560 - Increase the code coverage: DynamoCore/Models Folder I started adding just one test method TestOnRequestDispatcherBeginInvoke() for testing the DynamoModelEvents class. * DYN-2560 - Adding test cases for DynamoModelEvents Adding test cases for DynamoModelEvents * DYN-2560 - Adding test cases for DynamoModelEvents I added all the test cases for the all events in the DynamoModelEvent class, i just need to fix the last 6 of them. * DYN-2560 - Adding test cases for DynamoModelEventsArgs I added several test cases for the classes inside the DynamoModelEventsArgs.cs file. ZoomEventArgs TaskDialogEventArgs EvaluationCompletedEventArgs DynamoModelUpdateArgs FunctionNamePromptEventArgs PresetsNamePromptEventArgs ViewOperationEventArgs PointEventArgs WorkspaceEventArgs ModelEventArgs * DYN-2560 - Code Review Comments Based in the comment done by Aaron in the GitHub pull request, I added more description comments for the method TestTaskDialogEventArgs() and also I added comments for a local function * DYN-2560 - Code Review Comments 2 There was a spelling error in two methods for the word "Internally", then I fixed this error in the two places. * Python Engine Enum (#10618) * Cherrypick * Comments * Add unit test * Comments * Handle runtime table gaps on code block deletion (#10605) When the runtime table are built there is an implicit assumption that the code block ids are consecutive. However, that is not always the case, as the deletion of a procedure causes the deletion of its child code blocks, which may generate gaps in the id numbering. In order to make the code resiliant to these gaps, the runtime tables are sized based on the largest code block id, rather than in the amount of code blocks. * Validate ASM installations before loading (#10621) The check for the specific assemblies tbb.dll and tbbmalloc.dll is generalized to a full file list validation of detected ASM locations. This way, Dynamo is guarded against any incomplete/unusual ASM binary folders that other applications might include. The lists of files for each version were taken from LibG. They cannot be reused from LibG without involving major changes in the preloader, so the lists should be kept in sync as new major release of ASM occur. * SQ bug fix (#10622) * (1) Null reference bug fix from SQ dashboard * Add support for debug modes (#10603) * Add support for debug modes * Increase coverage of the Updates folder (#10628) * Add a test for the CrashPromptArgs class * Update DynamoCoreTests.csproj * Added NotificationObject tests * Updated the test, NotificationObject coverage at 100% * Changed some access modifiers * Added coverage for Updates/BinaryVersion * Added comments and fixed names * Added asserts for happy path, changed namespace and deleted a console function * Removed using * Revert "Removed using" This reverts commit 022823d. * Removed a change that should not be there * Added coverage for Updates folder * Tests for CPython3 Engine as well as for IronPython. * [WIP][FEEDBACK] Reduce test time by substantially reducing number of serialization tests. (#10624) * reduce number of serialization tests by factor 3~ * reduce wpf json serialization tests * Handle missing instance calling method statically (#10630) A code block node calling an instance method in its static form would make Dynamo crash if the instance was not provided. An example of this would be calling 'Curve.Patch();'. The cause of the issue was that the default argument was ultimately tried to be interpreted as a pointer. By avoiding that wrong conversion the engine is now able to surface the real problem as a human-readable warning. * Python3 Selection Under Debug Modes (#10629) * Cherrypick Python3 changes * Cleanup * Use Debug Modes * CleanUp * Rename Function * Clean Up * Do not use anouymous function as handler * Revert newer language change * Add adp analytics to Dynamo (#10576) * add ADPTracker register * Fix non-array item search for dot operation (#10633) When an array is passed to the dot operation, it needs to get an item to determine the actual class being processed. This was done in ArrayUtils.GetFirstNonArrayStackValue, but the function only checked the first item of the array and its descendants. This caused the dot operation to failed when passed an array that contained an empty array as its first item. In order to fix the problem, the function was changed to check for non-array items in the entire array. The function should stop as soon as the first non-array item is found. * Addressing some comments * some more comments * Update AssemblyInfo.cs * Update AssemblyInfo.cs * Marking the python node as modified when its engine property is modified Also updated some tests. * Remove unwanted check. * changes to test * Update AssemblySharedInfo.cs * Using python engine's AcquireLock to avoid deadlock. The deadlock was happening only when multiple PythonEngine's are initialized on a thread from 2 different test fixtures in the same run. The main difference is calling this function PythonEngine.BeginAllowThreads(). Also we do not want to initialze the python engine if it is already initialized. Co-authored-by: aparajit-pratap <aparajit.pratap@autodesk.com> Co-authored-by: Martin Misol Monzo <mmisol@users.noreply.github.com> Co-authored-by: Bruno Yorda <34453173+byorda-glb@users.noreply.github.com> Co-authored-by: Roberto T <61755417+RobertGlobant20@users.noreply.github.com> Co-authored-by: Aaron (Qilong) <173288704@qq.com> Co-authored-by: Ashish Aggarwal <aggarwal.ash@husky.neu.edu> Co-authored-by: Tibi <tiberiu.pinzariu@autodesk.com> Co-authored-by: Michael Kirschner <mjk.kirschner@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please Note:
DynamoRevitrepo will need to be cherry-picked into all the DynamoRevit Release branches that Dynamo supports. Contributors will be responsible for cherry-picking their reviewed commits to the other branches after aLGTMlabel is added to the PR.Purpose
@pinzart Please help on this, after merging the debug modes changes, I found it is not a singleton under DynamoModel or view model, do I have to deserialize it before I can use it in the code?
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@DynamoDS/dynamo
FYIs
@pinzart