Skip to content

Conversation

@delsner
Copy link
Member

@delsner delsner commented Jun 17, 2025

Motivation

Addresses #59.

Changes

We don't want to introduce a separate decorator as done in #60 with dy.lazy_rule.
Instead this PR instead lazily evaluates (group-)rules and gets rid of the column check which requires all symbols used inside the rule expression to be defined (including the schema class itself).

@delsner delsner self-assigned this Jun 17, 2025
@codecov
Copy link

codecov bot commented Jun 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (c25880e) to head (93364f3).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #64   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           41        41           
  Lines         2153      2143   -10     
=========================================
- Hits          2153      2143   -10     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@delsner delsner marked this pull request as ready for review June 17, 2025 20:07
@borchero borchero mentioned this pull request Jun 17, 2025
@borchero borchero linked an issue Jun 17, 2025 that may be closed by this pull request
Copy link
Member

@borchero borchero left a comment

Choose a reason for hiding this comment

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

I think we still have to remove/simplify the RuleImplementationError for coverage. Feel free to get rid of the entire comment; the implementation in the polars plugin is trivial (I already tried), no need for keeping this stuff around.

@delsner delsner requested a review from borchero June 18, 2025 14:54
Copy link
Member

@borchero borchero left a comment

Choose a reason for hiding this comment

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

Thanks! 🚀

@delsner delsner merged commit eb61574 into main Jun 18, 2025
18 checks passed
@delsner delsner deleted the lazily-evaluate-rules-2 branch June 18, 2025 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Eager evaluation of methods by dy.rule can lead to NameErrors

3 participants