dpc10ster / RJafroc

Artificial Intelligence: Evaluating AI, optimizing AI
19 stars 8 forks source link

Moving away from OpenXLSX dependency #54

Closed pwep closed 3 years ago

pwep commented 4 years ago

Dev has indicated that openxlsx is due to be removed from CRAN in a few days (25th September). This has implications for packages dependent on it.

Alternative packages are:

I'm going to try and rework the existing code to use readxl and writexl.

dpc10ster commented 4 years ago

I moved away from xlsx because of the Java dependence. I agree with your choice of readxl and writexl as replacements for openxlsx. I have merged your code using readxl and am currently testing it on my machine.

Thanks,

Dev

On Sep 20, 2020, at 05:30, Peter Phillips notifications@github.com wrote:

Dev has indicated that openxlsx https://cran.r-project.org/web/packages/openxlsx/index.html is due to be removed from CRAN in a few days (25th September). This has implications for packages dependent on it.

Alternative packages are:

xlsx https://cran.r-project.org/web/packages/xlsx/index.html depends on Java being installed, so is not preferred at this point. readxl https://cran.r-project.org/web/packages/readxl/index.html and writexl https://cran.r-project.org/web/packages/writexl/index.html appear to have no dependencies and look like a good fit. I'm going to try and rework the existing code to use readxl and writexl.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dpc10ster/RJafroc/issues/54, or unsubscribe https://github.com/notifications/unsubscribe-auth/AH4NJRDAL7PFBSTLRFUZ5GDSGXDSFANCNFSM4RTRDXQQ.

pwep commented 4 years ago

Pull request https://github.com/dpc10ster/RJafroc/pull/56 uses the writexl package, however openxlsx is still used for generating data files from UtilOutputReportExcelFileDBMH.R and UtilOutputReportExcelFileORH.R

writexl will only write one data frame per sheet. I tried merging data frames into a larger single data frame, but the mixing of types in columns meant defaulting to the character type.

Compare the outputs below - openxlsx on the left, a writexl version on the right. On the right-hand view there are green Excel warning triangles for cells containing character strings that Excel thinks should be typed as numbers.

Screenshot 2020-09-21 at 23 14 41

There is the option of continuing with openxlsx for now - in which case branch off the readxl,writexl code and park it, to be picked up at a later date. There is no need to introduce more packages than needed, if openxlsx is now available again.

Alternatively the format of the output file from UtilOutputReportExcelFileDBMH.R and UtilOutputReportExcelFileORH.R could be modified to contain more worksheets (one for each data frame).

dpc10ster commented 4 years ago

I am not that bothered by the green triangles, however ...

I like the idea of parking these changes for now, to be picked up later if openxlsx again develops problems.

So I will revert RJafroc to where it was before we started merging these changes and including the readxl and writexl packages. But ...

I am having problems reverting; my GitHub skills are limited. When I try to revert to the state before I started merging the recent pull requests, I get error messages like the following:

error: Reverting is not possible because you have unmerged files. hint: Fix them up in the work tree, and then use 'git add/rm ' hint: as appropriate to mark resolution and make a commit. fatal: revert failed

I have tried discarding, etc, but keep hitting this block.

Is it possible for you to issue a PR which undoes the recent changes? Or is there another way?

Dev

On Mon, Sep 21, 2020 at 6:27 PM Peter Phillips <notifications@github.com mailto:notifications@github.com> wrote:

Pull request #56 https://github.com/dpc10ster/RJafroc/pull/56 uses the writexl package, however openxlsx is still used for generating data files from UtilOutputReportExcelFileDBMH.R and UtilOutputReportExcelFileORH.R

writexl will only write one data frame per sheet. I tried merging data frames into a larger single data frame, but the mixing of types in columns meant defaulting to the character type.

Compare the outputs below - openxlsx on the left, a writexl version on the right. On the right-hand view there are green Excel warning triangles for cells containing character strings that Excel thinks should be typed as numbers.

https://user-images.githubusercontent.com/732331/93826960-3c7b6080-fc60-11ea-9e67-6d7ef96d5b74.png There is the option of continuing with openxlsx for now - in which case branch off the readxl,writexl code and park it, to be picked up at a later date. There is no need to introduce more packages than needed, if openxlsx is now available again.

Alternatively the format of the output file from UtilOutputReportExcelFileDBMH.R and UtilOutputReportExcelFileORH.R could be modified to contain more worksheets (one for each data frame).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dpc10ster/RJafroc/issues/54#issuecomment-696412091, or unsubscribe https://github.com/notifications/unsubscribe-auth/AH4NJRHKRRY5LVFFAC7LMTDSG7HNXANCNFSM4RTRDXQQ.

dpc10ster commented 3 years ago

I am going to close this and hope we do not have to revisit this later; but if we do, the history is here.