Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hotfix: fix export data #2141

Open
wants to merge 1 commit into
base: release/6.0
Choose a base branch
from
Open

Hotfix: fix export data #2141

wants to merge 1 commit into from
+44 −71

Conversation

bcb37
Copy link
Collaborator

@bcb37 bcb37 commented Nov 26, 2024 •
edited
Loading

This pr:

  • removes the queries to 'monitored_decision_point' and 'monitored_decision_point_log' and replaces multiple rows with different MarkExperimentPointTime values with a single row with MarkExperimentPointTime coming from the enrollment table
  • avoids including metrics columns that do not correspond to queries defined in the experiment
  • fixes the display of metric keys

@bcb37 bcb37 linked an issue Nov 26, 2024 that may be closed by this pull request
Fix export data and formatting for 6.0 #2132
Open
ppratikcr7
ppratikcr7 reviewed Nov 27, 2024
View reviewed changes
Copy link
Collaborator

@ppratikcr7 ppratikcr7 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@bcb37 So wanted to confirm one thing, you are now showing the attribute metric (timeSeconds) instead of the class (masteryWorkspace) for a grouped metric in the metrics column, right? Should we show the complete grouped metric (Class -> Key -> Attribute)? I have attached screenshot for other to see the difference. The old export gives 2 rows for each of the 2 students and the new one as per this PR gives 1 row for each of the 2 students. This is working perfectly as expected.

Screenshot 2024-11-27 at 6 38 28 PM

Also, didn't get the 2nd point of this PR description. Which metric column will not be shown? We already show the metrics that are used in the experiment. Can you please elaborate this scenario a bit. Thanks

@bcb37
Copy link
Collaborator Author

bcb37 commented Nov 27, 2024

Also, didn't get the 2nd point of this PR description. Which metric column will not be shown? We already show the metrics that are used in the experiment. Can you please elaborate this scenario a bit. Thanks

@ppratikcr7 it was including metrics columns from other experiments because the metrics list wasn't restricted to just the metrics in the current experiment. So I fixed that.
The screenshot is from a prod export of an experiment that does not have queries "Percent Graduated", "Swap", etc.

Screenshot 2024-11-27 at 2 42 34 PM

@ppratikcr7 ppratikcr7 self-requested a review November 29, 2024 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ppratikcr7 ppratikcr7 ppratikcr7 approved these changes

@VivekFitkariwala VivekFitkariwala Awaiting requested review from VivekFitkariwala

@danoswaltCL danoswaltCL Awaiting requested review from danoswaltCL

Assignees

@bcb37 bcb37

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix export data and formatting for 6.0
2 participants
@bcb37 @ppratikcr7