Skip to content

Don't include '...' to empty-dimensional matrices #174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jul 28, 2025

Conversation

MichaelChirico
Copy link
Contributor

Closes #141.

Finally understood the code well enough to offer this simple fix.

Crucially, we don't need to worry about omit='both' case because that implies both dimensions are non-empty (and, in fact, large). Also, passing NULL to rbind(), cbind(), just omits them, leading to the desired behavior without needing an else clause.

@flying-sheep
Copy link
Member

this one also has a conflict now!

@flying-sheep
Copy link
Member

Hmm, could you please add a test to check if they’re displayed correctly?

The example you gave in #141 should probably not show NAs when there’s no actual rows, if I understood correctly.

@MichaelChirico
Copy link
Contributor Author

MichaelChirico commented Jul 24, 2025

Appreciate your guidance on what a test should look like -- the main thing I had in mind when testing is to ensure there's no warning.

Beyond that, I added the simple test:

expect_match(repr(m), "A04[^A]*A07")

Which checks that A05 and A06 are elided and the repr() output goes from A04 to A07. I wanted to avoid issues of potentially-fragile reliance on whitespace, hence the regex instead of a full equality test on the repr() output.

@flying-sheep
Copy link
Member

Yeah, maybe just add a check that the repr string is just one line for the zero-rows column (i.e. only the headers)

@flying-sheep flying-sheep added this pull request to the merge queue Jul 28, 2025
@flying-sheep
Copy link
Member

beautiful, thank you!

Merged via the queue into IRkernel:main with commit 826281b Jul 28, 2025
2 checks passed
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.

Implict rows/cols>0 assumption in arr_parts_combine
2 participants