Skip to content

Conversation

@hcirellu
Copy link
Collaborator

This pull request adds support for matrix, array and %*% for integer64. Since there are no S3 methods for matrix and array, they are added.
In matrix and array the base functions are used. In order to display convinient warnings and errors that show the call (e.g. matrix(1:10, nrow=5) instead of base::matrix(...)) I use withCallingHandlers(). If there is a generic call, this is displayed, and if not, the S3-method call is displayed.
In order to display matrices and arrays consistently to R, str.integer64 is changed accordingly.
colSums.integer64 and rowSums.integer64 are refactored to throw R consistent error messages.
as.matrix.integer64 is added to coerce accordingly.
Since aperm.integer64 isn't exported, one has to use the generic. Thus it is refactored to use NextMethod().
The matrix multiplication uses a new helper function target_class_for_Ops(), which determines the class of the result. If a matrix multiplication of integer64 and complex is performed, I expect the result in complex. It also throws R consistent error messages for Ops. This helper is also intended to be used for other Ops methods like *.integer64 in the future. The remaining method checks the matrix dimensions and performs the calculation.

Closes #45

@hcirellu hcirellu marked this pull request as ready for review November 26, 2025 17:24
hcirellu and others added 4 commits December 13, 2025 10:17
Co-authored-by: Michael Chirico <michaelchirico4@gmail.com>
Co-authored-by: Michael Chirico <michaelchirico4@gmail.com>
try fix tests for ubuntu latest
@hcirellu

This comment was marked as outdated.

@hcirellu

This comment was marked as outdated.

@MichaelChirico

This comment was marked as outdated.

@MichaelChirico

This comment was marked as outdated.

Copy link
Collaborator

@MichaelChirico MichaelChirico left a comment

Choose a reason for hiding this comment

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

I think it's basically ready. Thanks!

@hcirellu
Copy link
Collaborator Author

hcirellu commented Jan 9, 2026

Thank you for the review!

@MichaelChirico MichaelChirico merged commit 90c3ae3 into r-lib:main Jan 9, 2026
8 checks passed
} else {
array(as.integer64(x), c(length(x), 1L), {if (!is.null(names(x))) list(names(x), NULL) else NULL})
}
array(x, c(length(x), 1L), if (!is.null(names(x))) list(names(x), NULL))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added this as.integer64, because I think it is good practice to assure that this method returns integer64, even though it might have been called directly internally without integer64.
But here the result would be inconsistent within the method, because if the input would be a non-integer64 matrix, it would be returned without converting to integer64.
Should I modify it to assure an integer64 return value or leave it as it is?
@MichaelChirico ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Generally I think it's overkill to call as.* again from within a .* S3 method -- if we get non-integer64 input, user is playing with fire :)

We have a few ugly checks in as.data.table.data.frame that are basically there for back-compatibility -- I'd rather the user be stuck debugging such cases:

https://github.com/Rdatatable/data.table/blob/a325db9e0c6a0cd1076c086d5fc17a548bccd1a5/R/as.data.table.R#L254-L261

@hcirellu hcirellu deleted the matrix_array_matmult branch January 12, 2026 09:04
@hcirellu hcirellu restored the matrix_array_matmult branch January 12, 2026 09:05
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.

matrix(<integer64>) strips the attribute

2 participants