feat: expose image attribute as expression #4848

Jay-ju · 2025-07-25T09:20:19Z

Changes Made

Related Issues

Checklist

Documented in API Docs (if applicable)
Documented in User Guide (if applicable)
If adding a new documentation page, doc is added to docs/mkdocs.yml navigation
Documentation builds and is formatted properly (tag @/ccmao1130 for docs review)

codecov · 2025-07-25T09:54:45Z

Codecov Report

❌ Patch coverage is 70.71429% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.29%. Comparing base (33884be) to head (17bdc90).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
src/daft-image/src/ops.rs	46.34%	22 Missing ⚠️
src/daft-image/src/series.rs	41.66%	7 Missing ⚠️
src/daft-schema/src/image_property.rs	77.77%	6 Missing ⚠️
src/daft-image/src/functions/attribute.rs	80.00%	5 Missing ⚠️
daft/__init__.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4848      +/-   ##
==========================================
+ Coverage   73.81%   74.29%   +0.47%     
==========================================
  Files         957      959       +2     
  Lines      124278   123335     -943     
==========================================
- Hits        91740    91635     -105     
+ Misses      32538    31700     -838

Files with missing lines	Coverage Δ
daft/expressions/expressions.py	`96.61% <100.00%> (+0.04%)`	⬆️
src/daft-core/src/array/image_array.rs	`97.43% <100.00%> (+0.29%)`	⬆️
src/daft-core/src/lit/conversions.rs	`60.41% <ø> (ø)`
src/daft-image/src/functions/mod.rs	`100.00% <100.00%> (ø)`
src/daft-schema/src/python/mod.rs	`100.00% <100.00%> (ø)`
daft/__init__.py	`25.00% <0.00%> (ø)`
src/daft-image/src/functions/attribute.rs	`80.00% <80.00%> (ø)`
src/daft-schema/src/image_property.rs	`77.77% <77.77%> (ø)`
src/daft-image/src/series.rs	`77.77% <41.66%> (-3.53%)`	⬇️
src/daft-image/src/ops.rs	`66.66% <46.34%> (-3.95%)`	⬇️

... and 27 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

universalmind303 · 2025-07-25T13:25:58Z

daft/expressions/expressions.py

@@ -5154,6 +5154,18 @@ def to_mode(self, mode: str | ImageMode) -> Expression:
        f = native.get_function_from_registry("to_mode")
        return Expression._from_pyexpr(f(self._expr, mode=image_mode))

+    def attribute(self, name: str) -> Expression:


since we know what attributes are available, use a Literal['width', 'height', 'channel', 'mode'] for better typings

sounds good

universalmind303 · 2025-07-25T13:27:02Z

src/daft-image/src/ops.rs

@@ -194,6 +195,31 @@ impl ImageOps for ImageArray {
            .collect();
        image_array_from_img_buffers(self.name(), &buffers, Some(mode))
    }
+
+    fn attribute(&self, attr: &str) -> DaftResult<DataArray<UInt32Type>> {


Can we use an Enum to model the attributes instead of the &str

universalmind303 · 2025-08-06T15:21:49Z

@Jay-ju what's the status on this PR? I think once those two small changes are implemented, we could merge this one in!

Jay-ju · 2025-08-14T09:51:48Z

@Jay-ju what's the status on this PR? I think once those two small changes are implemented, we could merge this one in!

@universalmind303 so sorry, I've been busy with other matters these few weeks. Some updates have been made here. Please take a look when you have time.

Jay-ju · 2025-08-14T11:34:43Z

code style is confused. run local success.

srilman · 2025-08-14T16:26:34Z

@Jay-ju how are you running pre-commit locally?

Jay-ju · 2025-08-15T01:51:37Z

@Jay-ju how are you running pre-commit locally?

@srilman
When executing make precommit locally, an error occurs. However, in the CI, if the comment # type: ignore[type-arg] in Embedding = np.typing.NDArray # type: ignore[type-arg] is deleted, an error will occur, so this comment is retained here.

If simply executing pre-commit run --all-files, it is successful locally, but an error occurs in the CI, and the reason for the error is not displayed.

srilman · 2025-08-18T20:38:03Z

Ok I think there is some slight difference in mypy rules between local and CI. Could you make the change you suggested to fix CI for now, even if it fails locally?

Jay-ju · 2025-08-26T09:39:01Z

Ok I think there is some slight difference in mypy rules between local and CI. Could you make the change you suggested to fix CI for now, even if it fails locally?

@srilman My current local version has run successfully. The currently failing unit test is an unstable unit test, and I have fixed it. Could you please check it again when you have time?

srilman

Overall, this looks good to me, but rather than an .image.attribute expression, wouldn't it make more sense to have expressions for each of the attributes themselves, like .image.width, .image.height, etc?

Jay-ju · 2025-08-28T03:39:02Z

Overall, this looks good to me, but rather than an .image.attribute expression, wouldn't it make more sense to have expressions for each of the attributes themselves, like .image.width, .image.height, etc?

@srilman
Actually, splitting it here has the same function as converging it to the attribute function. This was considered during the initial implementation. The main reason for not splitting it at that time was the hope that different types (images, videos, audio) could reuse the attribute method, and then obtain it based on the enumeration values of the function's input parameters; otherwise, it would be necessary to refer to the documentation to get this attribute.

srilman · 2025-08-28T03:49:34Z

@Jay-ju my main concern is that when looking at the list of available functions, via IDE tools, properties like the width, height, etc won't show up because they are potential arguments to .image.attribute. I'm ok with having an .image.attribute, but for visibility, I believe we should have .image.width ... as well

Jay-ju · 2025-08-28T08:31:41Z

@Jay-ju my main concern is that when looking at the list of available functions, via IDE tools, properties like the width, height, etc won't show up because they are potential arguments to .image.attribute. I'm ok with having an .image.attribute, but for visibility, I believe we should have .image.width ... as well

@srilman That sounds reasonable; it has been revised.

Jay-ju · 2025-09-01T05:48:42Z

@universalmind303 Can this be merged?？

srilman · 2025-09-05T04:09:39Z

@Jay-ju can you fix the style issue on CI?

Jay-ju · 2025-09-09T14:17:35Z

@Jay-ju can you fix the style issue on CI?

@srilman fixed it

github-actions bot added the feat label Jul 25, 2025

Jay-ju force-pushed the image_attribute branch from 6df8f84 to 32974ed Compare July 25, 2025 09:27

universalmind303 reviewed Jul 25, 2025

View reviewed changes

Jay-ju force-pushed the image_attribute branch 4 times, most recently from a72a431 to 367bfd9 Compare August 14, 2025 09:50

Jay-ju force-pushed the image_attribute branch from 367bfd9 to 07ea9c5 Compare August 14, 2025 11:03

Jay-ju force-pushed the image_attribute branch from 07ea9c5 to 338f55b Compare August 16, 2025 08:04

Jay-ju force-pushed the image_attribute branch 2 times, most recently from 7cff310 to 6d53c36 Compare August 26, 2025 02:27

Jay-ju force-pushed the image_attribute branch from f49d4bf to a4b303f Compare August 26, 2025 08:17

Jay-ju force-pushed the image_attribute branch from a4b303f to 7891081 Compare August 27, 2025 05:18

srilman reviewed Aug 27, 2025

View reviewed changes

Jay-ju force-pushed the image_attribute branch 2 times, most recently from 256fe15 to ac5885b Compare August 28, 2025 08:30

universalmind303 approved these changes Aug 28, 2025

View reviewed changes

Jay-ju added 3 commits September 9, 2025 22:00

feat: expose image attribute as expression

c3efbb3

expose more attribute

12155ca

format

f901f26

Jay-ju force-pushed the image_attribute branch from 1816465 to f901f26 Compare September 9, 2025 14:01

format

17bdc90

srilman merged commit bc9d370 into Eventual-Inc:main Sep 9, 2025
64 of 67 checks passed

feat: expose image attribute as expression #4848

feat: expose image attribute as expression #4848

Uh oh!

Conversation

Jay-ju commented Jul 25, 2025

Changes Made

Related Issues

Checklist

Uh oh!

codecov bot commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

universalmind303 Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

Jay-ju Jul 26, 2025

Choose a reason for hiding this comment

Uh oh!

universalmind303 Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

universalmind303 commented Aug 6, 2025

Uh oh!

Jay-ju commented Aug 14, 2025

Uh oh!

Jay-ju commented Aug 14, 2025

Uh oh!

srilman commented Aug 14, 2025

Uh oh!

Jay-ju commented Aug 15, 2025

Uh oh!

srilman commented Aug 18, 2025

Uh oh!

Jay-ju commented Aug 26, 2025

Uh oh!

srilman left a comment

Choose a reason for hiding this comment

Uh oh!

Jay-ju commented Aug 28, 2025

Uh oh!

srilman commented Aug 28, 2025

Uh oh!

Jay-ju commented Aug 28, 2025

Uh oh!

Jay-ju commented Sep 1, 2025

Uh oh!

srilman commented Sep 5, 2025

Uh oh!

Jay-ju commented Sep 9, 2025

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jul 25, 2025 •

edited

Loading