Add Analytics Coverage for Python Operations #11208

QilongTang · 2020-10-27T04:53:06Z

Please Note:

Before submitting the PR, please review How to Contribute to Dynamo
Dynamo Team will meet 1x a month to review PRs found on Github (Issues will be handled separately)
PRs will be reviewed from oldest to newest
If a reviewed PR requires changes by the owner, the owner of the PR has 30 days to respond. If the PR has seen no activity by the next session, it will be either closed by the team or depending on its utility will be taken over by someone on the team
PRs should use either Dynamo's default PR template or one of these other template options in order to be considered for review.
PRs that do not have one of the Dynamo PR templates completely filled out with all declarations satisfied will not be reviewed by the Dynamo team.
PRs made to the DynamoRevit repo 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 a LGTM label is added to the PR.

Purpose

Adding anaytics coverage for Python operations, way points added as part of https://git.autodesk.com/Dynamo/Analytics.NET/pull/22.

Declarations

Check these if you believe they are true

The codebase is in a better state after this PR
Is documented according to the standards
The level of testing this PR includes is appropriate
User facing strings, if any, are extracted into *.resx files
All tests pass using the self-service CI.
Snapshot of UI changes, if any.
Changes to the API follow Semantic Versioning and are documented in the API Changes document.
This PR modifies some build requirements and the readme is updated

Reviewers

@DynamoDS/dynamo

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

mjkkirschner · 2020-10-27T16:10:41Z

Is this attached to a specific jira task?

mjkkirschner · 2020-10-27T16:11:57Z

src/PythonMigrationViewExtension/PythonMigrationViewExtension.csproj

@@ -147,6 +147,10 @@
      <Name>PythonNodeModels</Name>
      <Private>False</Private>
    </ProjectReference>
+    <ProjectReference Include="..\NodeServices\DynamoServices.csproj">


whats added from this project?

Analytics is only available from logging, so in the future, everytime we want to track on extension layer (at least the OOTB ones developed by us), this will be a dependency in that case seems. We could expose APIs but unless we expose dashboards as well, APIs wont be useful I think

mjkkirschner · 2020-10-27T16:12:19Z

src/Libraries/PythonNodeModelsWpf/ScriptEditorWindow.xaml.cs

@@ -209,14 +218,24 @@ private void OnMoreInfoClicked(object sender, RoutedEventArgs e)

        private void OnEngineChanged(object sender, System.Windows.Controls.SelectionChangedEventArgs e)
        {
-            if (CachedEngine != nodeModel.Engine || originalScript != editText.Text) { nodeWasModified = true; }
+            if (CachedEngine != nodeModel.Engine || originalScript != editText.Text)
+            {


QilongTang · 2020-10-27T16:12:36Z

Is this attached to a specific jira task?

https://jira.autodesk.com/browse/DYN-3024

QilongTang · 2020-10-28T18:21:53Z

Seems https://github.com/DynamoDS/Dynamo/blob/master/test/DynamoCoreTests/AnalyticsTests.cs#L76 already covers regular event tracking on analytics side so new analytics code can be added without adding unit test unless a new type of type introduced. @pinzart What do you think?

QilongTang · 2020-10-28T18:23:33Z

src/PythonMigrationViewExtension/Differ/SideBySideViewModel.cs

@@ -8,7 +8,7 @@ public class SideBySideViewModel : IDiffViewViewModel
        public SideBySideDiffModel DiffModel { get; set; }
        public DiffPaneModel AfterPane { get { return DiffModel.NewText; } }
        public DiffPaneModel BeforePane { get { return DiffModel.OldText; } }
-        private bool HasChanges { get { return DiffModel.NewText.HasDifferences | DiffModel.OldText.HasDifferences; } }
+        public bool HasChanges { get { return DiffModel.NewText.HasDifferences | DiffModel.OldText.HasDifferences; } }


@mmisol @aparajit-pratap @mjkkirschner I dont recall why we specifically made this property private, but I would like to expose it on interface level and add analytics coverage so we know not only user clicked accept or Reject but also if there are any real code changes.

pinzart90 · 2020-10-29T14:29:46Z

Seems https://github.com/DynamoDS/Dynamo/blob/master/test/DynamoCoreTests/AnalyticsTests.cs#L76 already covers regular event tracking on analytics side so new analytics code can be added without adding unit test unless a new type of type introduced. @pinzart What do you think?

Existing unit tests should be good enough

pinzart90

LGTM

zeusongit

LGTM

QilongTang · 2020-10-29T17:26:35Z

Merging. @mjkkirschner I am thinking to actually cherry-picking this into RC2.9.0 so we have Python3 feature adoption baseline earlier. Any concerns?

* Initial Commit * Analytics For Migration Assistant * More changes * Update Code Sequence * Add Analytics for Python Evaluation * Clean Up * More comments * Updates * Updates

QilongTang added 2 commits October 27, 2020 00:52

Initial Commit

4a99a92

Analytics For Migration Assistant

dde018e

mjkkirschner reviewed Oct 27, 2020

View reviewed changes

QilongTang added 4 commits October 27, 2020 14:39

Merge branch 'master' into AddingAnalyticsCoverageForPythonEditor

1281d0b

More changes

0988827

Update Code Sequence

ab6d4e1

Add Analytics for Python Evaluation

5df3c8d

QilongTang changed the title ~~Add Analytics Coverage for Python Editor~~ Add Analytics Coverage for Python Operations Oct 28, 2020

QilongTang requested a review from pinzart October 28, 2020 17:07

QilongTang and others added 2 commits October 28, 2020 14:12

Merge branch 'master' into AddingAnalyticsCoverageForPythonEditor

9188d6f

Clean Up

b16e4bd

QilongTang commented Oct 28, 2020

View reviewed changes

More comments

520a371

QilongTang marked this pull request as ready for review October 28, 2020 20:24

Merge branch 'master' into AddingAnalyticsCoverageForPythonEditor

5585345

pinzart90 approved these changes Oct 29, 2020

View reviewed changes

QilongTang requested a review from zeusongit October 29, 2020 15:01

zeusongit approved these changes Oct 29, 2020

View reviewed changes

QilongTang added 3 commits October 29, 2020 13:11

Merge branch 'master' into AddingAnalyticsCoverageForPythonEditor

5749b50

Updates

d0e9eb0

Updates

a30bb97

QilongTang merged commit 67f7976 into master Oct 29, 2020

QilongTang deleted the AddingAnalyticsCoverageForPythonEditor branch October 29, 2020 17:26

QilongTang mentioned this pull request Oct 30, 2020

Add Analytics Coverage for Python Operations (#11208) #11223

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Analytics Coverage for Python Operations #11208

Add Analytics Coverage for Python Operations #11208

QilongTang commented Oct 27, 2020 •

edited

Loading

mjkkirschner commented Oct 27, 2020

mjkkirschner Oct 27, 2020

QilongTang Oct 27, 2020 •

edited

Loading

mjkkirschner Oct 27, 2020

QilongTang commented Oct 27, 2020

QilongTang commented Oct 28, 2020

QilongTang Oct 28, 2020

pinzart90 commented Oct 29, 2020

pinzart90 left a comment

zeusongit left a comment

QilongTang commented Oct 29, 2020

Add Analytics Coverage for Python Operations #11208

Add Analytics Coverage for Python Operations #11208

Conversation

QilongTang commented Oct 27, 2020 • edited Loading

Purpose

Declarations

Reviewers

FYIs

mjkkirschner commented Oct 27, 2020

mjkkirschner Oct 27, 2020

Choose a reason for hiding this comment

QilongTang Oct 27, 2020 • edited Loading

Choose a reason for hiding this comment

mjkkirschner Oct 27, 2020

Choose a reason for hiding this comment

QilongTang commented Oct 27, 2020

QilongTang commented Oct 28, 2020

QilongTang Oct 28, 2020

Choose a reason for hiding this comment

pinzart90 commented Oct 29, 2020

pinzart90 left a comment

Choose a reason for hiding this comment

zeusongit left a comment

Choose a reason for hiding this comment

QilongTang commented Oct 29, 2020

QilongTang commented Oct 27, 2020 •

edited

Loading

QilongTang Oct 27, 2020 •

edited

Loading