YAXArrays.jl icon indicating copy to clipboard operation
YAXArrays.jl copied to clipboard

Infer outputtype by Base.return_types

Open felixcremer opened this issue 4 weeks ago • 3 comments

This should make broadcasting mixed type arrays much more robust. I had the problem that when I broadcasted two YAXArrays with Union{Missing, UInt8} the outtype was determined as Missing and then the actual results where not able to be written into the Array.

I added a test for such a case.

I am wondering whether we should also make this the default for xmap outtypes in general.

felixcremer avatar Nov 12 '25 16:11 felixcremer

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 68.55%. Comparing base (7ab2a92) to head (1317d32). :warning: Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #562      +/-   ##
==========================================
+ Coverage   67.34%   68.55%   +1.21%     
==========================================
  Files          15       15              
  Lines        2205     2204       -1     
==========================================
+ Hits         1485     1511      +26     
+ Misses        720      693      -27     
Files with missing lines Coverage Δ
src/DAT/broadcast.jl 85.71% <100.00%> (+85.71%) :arrow_up:

... and 1 file 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.

codecov[bot] avatar Nov 12 '25 16:11 codecov[bot]

I don't understand why this change is not hit by codecov

felixcremer avatar Nov 12 '25 18:11 felixcremer

I found it, we didn't include the broadcast tests. They are now enabled.

felixcremer avatar Nov 12 '25 18:11 felixcremer

@felixcremer ? merge? or do we still want?

However, a quick test for something a function like f(x) = rand() > 0.5 ? Float64(1) : Float32(1) and then f.(cube) would be nice to add.

lazarusA avatar Nov 28 '25 10:11 lazarusA

is this related to #540 ?

lazarusA avatar Nov 28 '25 19:11 lazarusA

is this related to #540 ?

No, this was a user error on my part and we could ease this a bit with a better error message.

felixcremer avatar Nov 28 '25 22:11 felixcremer