Skip to content

Additional polish on show methods#345

Merged
johnkerl merged 7 commits intokerl/sc-12282-array-schema-showfrom
de/sc-12282/show_methods
Jan 10, 2022
Merged

Additional polish on show methods#345
johnkerl merged 7 commits intokerl/sc-12282-array-schema-showfrom
de/sc-12282/show_methods

Conversation

@eddelbuettel
Copy link
Copy Markdown
Contributor

PR #342 is really good and gets us almost to the finish line of not relying on core code (to stdout) for object display and adding a numer of missing show() methods.

It tickled an error an real penguins data set (with NA values, as opposed to the cleansed one in cloud use) which I fixed that. While at it, I also made the code a little tighter and more idiomatic (and removed use of try). I apologise for the whitespace changes but I am quite used to how ESS typesets in base R defaults and this brings methods in further to the left.

Please have a look with your test arrays, it it is looking ok on the ones I tried.

(The last commit was needed or else one test on UINT32 would balk here. I am not sure how that didn't come up when we looked at the PR that brought it in. I should also have caught the use of tiledb_dense there which we try to phase out / will remove next month.)

@shortcut-integration
Copy link
Copy Markdown

@eddelbuettel
Copy link
Copy Markdown
Contributor Author

eddelbuettel commented Jan 9, 2022

CI is failing on Windows due to network flakyness for the (forced) installation of qpdf(which we don't even use/need). On re-run one of three passes, will probably hit the re-run button again in an hour or so. It's all fine on Azure for the other two OS.

A re-run of CI at GitHub resolved the flakyness caused earlier by network / remote site issues.

@johnkerl
Copy link
Copy Markdown
Contributor

Looks good to me -- thanks @eddelbuettel !!

@johnkerl johnkerl merged commit 28f91ad into kerl/sc-12282-array-schema-show Jan 10, 2022
@johnkerl johnkerl changed the title Additinal polish on show methods Additional polish on show methods Jan 10, 2022
@eddelbuettel
Copy link
Copy Markdown
Contributor Author

My penny-dropping mechanism had a malfunction last week so I was overly slow in groking what you needed. Once I did it was just a question of suggesting a different touch here or there.

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.

2 participants