feat: expose image attribute as expression

[Jay-ju]

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[bot]]

Codecov Report

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

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

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%)	:arrow_up:
src/daft-core/src/array/image_array.rs	97.43% <100.00%> (+0.29%)	:arrow_up:
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%)	:arrow_down:
src/daft-image/src/ops.rs	66.66% <46.34%> (-3.95%)	:arrow_down:

... and 27 files with indirect coverage changes

:rocket: New features to boost your workflow:

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

[universalmind303]

@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]

@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]

code style is confused. run local success.

[srilman]

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

[Jay-ju]

@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]

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]

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?

[Jay-ju]

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]

@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]

@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]

@universalmind303 Can this be merged?？

[srilman]

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

[Jay-ju]

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

@srilman fixed it