R6 classes - update for wrappers #82
Conversation
* only try to unmerge cells when merged cells are found * simplify function names * simplify rows and columns setting * simplify object names * combine documentation
@JanMarvin should be happy about this
these values can be predefined in fields
Related to #57
related to #57
the plural makes a bit more sense
make path and create directory -- wow all in 1 step...
more to come
|
Regarding the function names: I was just under the impression, that it might be a bit confusing, if many function start with Ideally I would want all piped and wrapped functions to be named identical (otherwise we create even more confusion). wb <- workbook()
wb_add_worksheet(wb, "Sheet1")
wb$add_worksheet("Sheet1") |
There was a problem hiding this comment.
Looks good, a big step, but of course I'm still nitpicking.
We should also add unit tests for the wrapper functions (just to be sure, but increasing coverage is another task of ours).
If you want to, feel free to go through my comments, change what you think is useful and merge afterwards. We can always cleanup in additional pull requests.
|
Oh and do you know why the lintr is complaining about no visible global functions? It looks a lot like false positives, but I' curious. |
No idea. I can see the functions. We must have confused it too much. |
|
@JanMarvin, okay, with those updates I think this might be in a better shape to be merged. The updates for #84 are branched from this branch and those should make up for my decreasing of the test coverage... |
|
hooray, merge when ready! 🥳 |
updates for some progress in #75
These are mostly the easy ones -- moving the code around.
Changes outside of these are for the updated named. I'm not too worried about how some of the tests look right now and we can make some changes between piped operations and calling methods directly from the
R6objects (e.g., moving fromwb <- wb_add_worksheet(wb, "sheet")towb <- wb_workbook() %>% wb_add_worksheet("sheet")or justwb$addWorksheet()