Get MF using the new dsi transformations #1877
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
|
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
Merged
theyostalservice
force-pushed
the
patricky/get-mf-using-new-transformations
branch
from
edd4e74 to
a48c2f4
Compare
theyostalservice
force-pushed
the
patricky/bump-dsi-version-hooray
branch
from
3bc07c3 to
ec37377
Compare
theyostalservice
changed the base branch from
patricky/bump-dsi-version-hooray
to
graphite-base/1877
theyostalservice
force-pushed
the
graphite-base/1877
branch
from
ec37377 to
bdcdd56
Compare
theyostalservice
force-pushed
the
patricky/get-mf-using-new-transformations
branch
from
a48c2f4 to
50c1434
Compare
theyostalservice
changed the base branch from
graphite-base/1877
to
patricky/bump-dsi-version-hooray
theyostalservice
force-pushed
the
patricky/get-mf-using-new-transformations
branch
from
50c1434 to
1bb321e
Compare
theyostalservice
force-pushed
the
patricky/bump-dsi-version-hooray
branch
from
bdcdd56 to
0fd70d9
Compare
theyostalservice
force-pushed
the
patricky/get-mf-using-new-transformations
branch
from
1bb321e to
68f8985
Compare
theyostalservice
changed the base branch from
patricky/bump-dsi-version-hooray
to
graphite-base/1877
theyostalservice
added a commit
that referenced
this pull request
Oct 3, 2025
Version BUUUUUUUMP This PR is just here to pull in the new transformations and such from dsi. Early commits in this PR will be pinned to commits while I'm working on feeding dbt-labs/dbt-semantic-interfaces#436 to the merge-asaurus. Snapshot updates reflect the fact that now(*) we produce more metrics to replace measures, reflecting the changes in structure for the new user YAML. (*) "now" is true, but ONLY IN TESTS. Because of a transformation rule that exists only in MF right now, we don't use the list of transformations directly from dsi. See upstack #1877 for the PR that brings these changes to prod. OTH, tests DO run on the default list of metric transformations from dsi, so the snapshots needed more immediate updating.
theyostalservice
force-pushed
the
patricky/get-mf-using-new-transformations
branch
from
68f8985 to
c718dfe
Compare
theyostalservice
force-pushed
the
graphite-base/1877
branch
from
90b07c1 to
5b55953
Compare
theyostalservice
force-pushed
the
patricky/get-mf-using-new-transformations
branch
from
c718dfe to
d0c497e
Compare
theyostalservice
commented
Oct 3, 2025
Comment on lines
+37
to
+42
| # These individual rules come from rule_set.convert_legacy_measures_to_metrics_rules, but | ||
| # dsi requires AddInputMetricMeasuresRule, and metricflow requires that we do NOT run that rule | ||
| # as it is incompatible with a parser like dbt-core that pre-populates input measures. | ||
| CreateProxyMeasureRule(), | ||
| FlattenSimpleMetricsWithMeasureInputsRule(), | ||
| ReplaceInputMeasuresWithSimpleMetricsTransformationRule(), |
Contributor
Author
There was a problem hiding this comment.
if we're properly motivated, I can refactor this in dsi to make this cleaner here, but for now, I'd really like to get this in and working today.
courtneyholcomb
approved these changes
Oct 3, 2025
Contributor
courtneyholcomb
left a comment
courtneyholcomb
left a comment
There was a problem hiding this comment.
curious what happened with AddInputMetricMeasuresRule / if that's an issue? But this looks good
Contributor
Author
|
It took me a bit of digging to figure this out, but here's what I know about the current situation:
So, imo, the most reasonable clean-up outcomes we could pursue here:
|
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.
Depends on #1876
Upstream PR bumps the dsi version, implicitly updating tests to using the default ruleset from dsi. We can't do that for production MF rn because MF uses an extra transformation internally, so this PR adds all the newer built in PRs from dsi to metricflow-semantics/metricflow_semantics/model/dbt_manifest_parser.py to be used in real circumstances.
(If time was less limited before coalesce, I'd prefer to move the rule upstream if possible, but I don't want to pick up extra work before code freeze.)
Edit: There's also a rule (AddInputMeasuresToMetrics) that only works in dsi. It probably merits a refactor/rewrite later to clean up the usage code here, but I'd rather prioritize getting this updated here for now.