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.

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

Fix python model parsing from meder #9162

Merged
merged 5 commits into from
Jan 31, 2024

Conversation

ChenyuLInx
Copy link
Contributor

@ChenyuLInx ChenyuLInx commented Nov 28, 2023

resolves #6976
Bring #7740 over finish line by adding a unittest. Thanks @mederka for adding it!

@runleonarun adding Copilot summary

This pull request primarily focuses on fixing the parsing of f-strings in Python models. The changes include adding support for dbt function calls in f-strings, modifying test cases to include f-string usage, and adding a new test case specifically for f-string configuration.

Fixes and improvements:

Changes to the Python model parser:

  • core/dbt/parser/models.py: Added support for dbt function calls in f-strings. This change allows the parser to correctly interpret dbt function calls that are embedded within f-strings.

Changes to the test cases:

  • tests/unit/test_parser.py: Modified the model function to include f-string usage. This change tests the parser's ability to correctly interpret f-strings.
  • tests/unit/test_parser.py: Added a new Python model that uses f-strings. This model is used in a new test case that verifies the correct parsing of f-strings.
  • tests/unit/test_parser.py: Modified the test_python_model_parse function to include a new reference argument that uses f-strings. This change tests the parser's ability to correctly interpret reference arguments within f-strings.
  • tests/unit/test_parser.py: Added a new test case test_python_model_f_string_config to verify the correct parsing of f-string configuration in Python models.

@ChenyuLInx ChenyuLInx requested a review from a team as a code owner November 28, 2023 18:21
@cla-bot cla-bot bot added the cla:yes label Nov 28, 2023
@@ -797,6 +797,7 @@ def model(dbt, session):
a_dict = {'test2': dbt.ref('test2')}
df5 = {'test2': dbt.ref('test3')}
df6 = [dbt.ref("test4")]
f"{dbt.ref('test5')}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jtcohen6 @graciegoheen With this change, now this syntax is supported to add a ref.
I don't feel like it is a bad thing, just a change

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok we should just make sure our docs our updated cc: @matthewshaver

Copy link

codecov bot commented Nov 28, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (eecaee1) 86.96% compared to head (2ea0538) 86.99%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9162      +/-   ##
==========================================
+ Coverage   86.96%   86.99%   +0.03%     
==========================================
  Files         187      187              
  Lines       24953    24957       +4     
==========================================
+ Hits        21700    21712      +12     
+ Misses       3253     3245       -8     
Flag Coverage Δ
integration 84.43% <25.00%> (+0.09%) ⬆️
unit 63.25% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@graciegoheen graciegoheen added the user docs [docs.getdbt.com] Needs better documentation label Jan 16, 2024
Copy link
Member

@aranke aranke left a comment

Choose a reason for hiding this comment

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

LGTM!

core/dbt/parser/models.py Show resolved Hide resolved
@ChenyuLInx ChenyuLInx merged commit 8717828 into main Jan 31, 2024
51 checks passed
@ChenyuLInx ChenyuLInx deleted the fix-python-model-parsing-from-meder branch January 31, 2024 22:38
@FishtownBuildBot
Copy link
Collaborator

Opened a new issue in dbt-labs/docs.getdbt.com: dbt-labs/docs.getdbt.com#4838

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes user docs [docs.getdbt.com] Needs better documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-2122] [Bug] project variables are None if dbt.config.get is called within string interpolation
5 participants