bmerge() needs to handle mixed complex-(int,dbl,i64) joins explicitly

[MichaelChirico]

Follow-up to #6603.

We have this comment that's currently incorrect:

data.table/R/bmerge.R

Line 100 in 546259d

# just integer and double left

That's because complex is an ORDERING_TYPE:

data.table/R/setkey.R

Line 150 in 546259d

ORDERING_TYPES = c('logical', 'integer', 'double', 'complex', 'character')

At a glance, we should:

1. Treat complex the same as double iff all(Im(x) == 0)
2. Error if any(Im(x) != 0)

To facilitate spotting this kind of gap in the future, I made this:

Spreadsheet: https://docs.google.com/spreadsheets/d/1RhNhXRA_5frgeqF9_ZS0Sll94RVs0iFgb5u7DBrgpaQ/edit?usp=sharing

@tdhock, what do you think would be a good place to keep this table as documentation? The joins vignette?

[tdhock]

wow nice table!!! yes the joins vignette seems appropriate.
thanks for your detailed analysis, this is impressive.

[tdhock]

the table is symmetric so maybe change the upper triangle to white/blank to emphasize that?

[MichaelChirico]

I should probably introduce asymmetry, because the resulting value _can_ depend on ordering, e.g. character-factor vs factor-character.

…

On Thu, Nov 28, 2024, 5:15 AM Toby Dylan Hocking ***@***.***> wrote: the table is symmetric so maybe change the upper triangle to white/blank to emphasize that? — Reply to this email directly, view it on GitHub <#6627 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB2BA5LDA3N6NSHB6VN7FPL2C4JP5AVCNFSM6AAAAABST43GZ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMBWGEYDKNBVGU> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[Abhishek2634]

Hii, @tdhock i would like to work on this issue, please assign me.

[venom1204]

@MichaelChirico i guess changing the
mergetype () function would be enough, i mean adding the condition to check wheather the complex should be treated as double or rejected in the mergetype function would be enough for this .what you think

[MichaelChirico]

There is existing #6664 awaiting review, feel free to comment.

(FYI you can look at the comment thread and it will link existing issues/PRs that link back to this issue; also, the issue page will show the assignee)

[venom1204]

hi
@Abhishek2634 are you still working on it? as it was assigned to you but no progress is done after the pr

[MichaelChirico]

Abhishek is waiting for my review on #6664 as I understand it. Haven't gotten to it yet.