Skip to content

Allow lazy python values#202

Merged
evinism merged 13 commits intomainfrom
evin/python_lazy
Sep 24, 2025
Merged

Allow lazy python values#202
evinism merged 13 commits intomainfrom
evin/python_lazy

Conversation

@evinism
Copy link
Copy Markdown
Owner

@evinism evinism commented Sep 23, 2025

This should allow us to not recursively convert the whole of the data parameter to values, and should mostly be transparent from both the producer and internal parts of MistQL.

There are two main changes here:

  1. Moving builtins to utilize dedicated/dunder methods for indexing, sizing, and a few other operations.
  2. Creating a lazy value type that dynamically builds the underlying data based on access patterns

I believe there are some pathological cases in which this could technically add compute, but they're very rare and have a bounded performance hits.

I also need to consist-ify the type to return in the dunder/special methods.

@netlify
Copy link
Copy Markdown

netlify Bot commented Sep 23, 2025

Deploy Preview for mistql canceled.

Name Link
🔨 Latest commit e843cf4
🔍 Latest deploy log https://app.netlify.com/projects/mistql/deploys/68d4662a28eb2a00076fd5d6

@evinism evinism changed the title Allow lazy python value conversion Allow lazy python values Sep 23, 2025
@evinism evinism requested a review from Copilot September 23, 2025 23:20
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces lazy evaluation for Python values in MistQL to optimize performance by deferring conversion of nested data structures until accessed. The main goal is to avoid recursively converting entire data parameters to RuntimeValues upfront.

Key Changes

  • Added lazy evaluation option to RuntimeValue creation with new LazyRuntimeValue class
  • Modified builtins to use dunder methods (len, iter) and dedicated methods (access, index) for operations
  • Updated MistQLInstance to support lazy evaluation configuration

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
py/mistql/runtime_value.py Core implementation of lazy evaluation with new LazyRuntimeValue class and dunder methods
py/mistql/builtins.py Updated builtin functions to use new dunder methods instead of direct .value access
py/mistql/instance.py Added lazy parameter to MistQLInstance constructor with default True
py/mistql/gardenwall.py Modified input_garden_wall to accept lazy parameter
py/tests/test_shared.py Updated tests to use MistQLInstance and test both lazy and non-lazy modes
py/tests/test_lazy_value.py Comprehensive test suite for lazy value functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread py/mistql/runtime_value.py Outdated
Comment thread py/mistql/builtins.py Outdated
Comment thread py/mistql/builtins.py Outdated
Comment thread py/mistql/builtins.py
@evinism evinism requested a review from Copilot September 24, 2025 00:36
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +405 to +407
return RuntimeValue.of(self.value[index])
else:
return RuntimeValue.of(self.value[index:index_two])
Copy link

Copilot AI Sep 24, 2025

Choose a reason for hiding this comment

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

This line creates a non-lazy RuntimeValue even when the parent is lazy, which defeats the purpose of lazy evaluation. It should pass the lazy flag: return RuntimeValue.of(self.value[index], lazy=True)

Suggested change
return RuntimeValue.of(self.value[index])
else:
return RuntimeValue.of(self.value[index:index_two])
return RuntimeValue.of(self.value[index], lazy=self.is_lazy())
else:
return RuntimeValue.of(self.value[index:index_two], lazy=self.is_lazy())

Copilot uses AI. Check for mistakes.
if index_two is None:
return RuntimeValue.of(self.value[index])
else:
return RuntimeValue.of(self.value[index:index_two])
Copy link

Copilot AI Sep 24, 2025

Choose a reason for hiding this comment

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

This line creates a non-lazy RuntimeValue for slices even when the parent is lazy. It should pass the lazy flag: return RuntimeValue.of(self.value[index:index_two], lazy=True)

Suggested change
return RuntimeValue.of(self.value[index:index_two])
return RuntimeValue.of(self.value[index:index_two], lazy=self.is_lazy())

Copilot uses AI. Check for mistakes.
Comment thread py/mistql/runtime_value.py Outdated
if not lazy:
return RuntimeValue(
RuntimeValueType.Object,
{key: RuntimeValue.of(value[key]) for key in value},
Copy link

Copilot AI Sep 24, 2025

Choose a reason for hiding this comment

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

This line in the non-lazy path is missing the lazy parameter. It should be RuntimeValue.of(value[key], lazy) to maintain consistency with the lazy parameter passed to the parent.

Suggested change
{key: RuntimeValue.of(value[key]) for key in value},
{key: RuntimeValue.of(value[key], lazy) for key in value},

Copilot uses AI. Check for mistakes.
@evinism
Copy link
Copy Markdown
Owner Author

evinism commented Sep 24, 2025

Preliminary tests show substantial performance gains (~6x over a simple query on nobel prizes dataset)

@evinism evinism merged commit d625ea1 into main Sep 24, 2025
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants