chore: Adding telemetry for the dataset metadata. This one is specially for … #1917
Conversation
…adding count of the columns.
saileshbaidya
requested review from
svotaw,
memoryz,
eisber,
imatiach-msft and
mhamilton723
as code owners
|
Hey @saileshbaidya 👋! We use semantic commit messages to streamline the release process. Examples of commit messages with semantic prefixes:
To test your commit locally, please follow our guild on building from source. |
saileshbaidya
changed the title
|
/azp run |
|
Commenter does not have sufficient privileges for PR 1917 in repo microsoft/SynapseML |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…eshbaidya/SynapseML_SB into saibai/TelemetryTask1893417
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Codecov Report
@@ Coverage Diff @@
## master #1917 +/- ##
==========================================
+ Coverage 86.77% 86.81% +0.03%
==========================================
Files 301 301
Lines 15677 15783 +106
Branches 815 848 +33
==========================================
+ Hits 13604 13702 +98
- Misses 2073 2081 +8
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -12,10 +12,11 @@ import scala.collection.mutable | |||
| case class SynapseMLLogInfo(uid: String, | |||
| className: String, | |||
| method: String, | |||
| buildVersion: String) | |||
| buildVersion: String, | |||
| columns: Int = -1) | |||
There was a problem hiding this comment.
In scala we use Option types to denote something that can be there or not. You can add this without changing much by also adding a different "this" method so you can just pass in columns as a non-optioned int too
There was a problem hiding this comment.
Will do. Agree, better to use language features.
There was a problem hiding this comment.
I have sent out new iteration with the changes @mhamilton723.
| def logFit[T](f: => T, columns: Int = -1): T = { | ||
| logVerb("fit", f, columns) | ||
| } | ||
|
|
||
| def logTrain[T](f: => T): T = { | ||
| logVerb("train", f) | ||
| def logTrain[T](f: => T, columns: Int = -1): T = { | ||
| logVerb("train", f, columns) | ||
| } | ||
|
|
||
| def logTransform[T](f: => T): T = { | ||
| logVerb("transform", f) | ||
| def logTransform[T](f: => T, columns: Int = -1): T = { | ||
| logVerb("transform", f, columns) | ||
| } |
There was a problem hiding this comment.
Unfortunately we still should probabbly get rid of this -1 default arg here. Please remove the default arg from this function as it should be provided in all cases right?
There was a problem hiding this comment.
Did that. Please check.
| protected def logBase(methodName: String, columns:Int = -1): Unit = { | ||
| logBase(SynapseMLLogInfo( | ||
| uid, | ||
| getClass.toString, | ||
| methodName, | ||
| BuildInfo.version, | ||
| if (columns == -1) None else Some(columns))) |
There was a problem hiding this comment.
Maybe this can just take in an Option of int too because it isnt used much
There was a problem hiding this comment.
Did that. Please check.
…eshbaidya/SynapseML_SB into saibai/TelemetryTask1893417
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…for a log api logBase call
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…adding count of the columns.
Related Issues/PRs
Task: 2348953
Close #2348953
What changes are proposed in this pull request?
Adding the dataset schema column count to the existing logs.
Briefly describe the changes included in this Pull Request.
How is this patch tested?
I am going to run the existing tests that calls the logging apis.
Does this PR change any dependencies?
Does this PR add a new feature? If so, have you added samples on website?
website/docs/documentationfolder.Make sure you choose the correct class
estimators/transformersand namespace.DocTablepoints to correct API link.yarn run startto make sure the website renders correctly.<!--pytest-codeblocks:cont-->before each python code blocks to enable auto-tests for python samples.WebsiteSamplesTestsjob pass in the pipeline.