-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
unifir: A Unifying API for Working with Unity in R #521
Comments
Thanks for submitting to rOpenSci, our editors and @ropensci-review-bot will reply soon. Type |
While it's not relevant to this issue, I do wonder if |
Checks for unifir (v0.1.0)git hash: 8e9ea053
Important: All failing checks above must be addressed prior to proceeding Package License: MIT + file LICENSE 1. Statistical PropertiesThis package features some noteworthy statistical properties which may need to be clarified by a handling editor prior to progressing. Details of statistical properties (click to open)
The package has:
Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages
All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the The final measure (
1a. Network visualisationClick to see the interactive network visualisation of calls between objects in package 2.
|
name | conclusion | sha | date |
---|---|---|---|
lint | success | 8e9ea0 | 2022-03-24 |
pages build and deployment | success | 16409d | 2022-03-24 |
pkgdown | success | 8e9ea0 | 2022-03-24 |
R-CMD-check | success | 8e9ea0 | 2022-03-24 |
test-coverage | success | 8e9ea0 | 2022-03-24 |
3b. goodpractice
results
R CMD check
with rcmdcheck
rcmdcheck found no errors, warnings, or notes
Test coverage with covr
Package coverage: 86.83
Cyclocomplexity with cyclocomp
The following functions have cyclocomplexity >= 15:
function | cyclocomplexity |
---|---|
action | 26 |
find_unity | 16 |
Static code analyses with lintr
lintr found the following 10 potential issues:
message | number of times |
---|---|
Lines should not be more than 80 characters. | 10 |
Package Versions
package | version |
---|---|
pkgstats | 0.0.3.94 |
pkgcheck | 0.0.2.275 |
Editor-in-Chief Instructions:
Processing may not proceed until the items marked with ✖️ have been resolved.
@ropensci-review-bot assign @melvidoni as editor |
I'm sorry human, I don't understand that. You can see what commands I support by typing:
|
@ropensci-review-bot assign @melvidoni as editor |
Assigned! @melvidoni is now the editor |
@ropensci-review-bot seeking reviewers |
Please add this badge to the README of your package repository: [![Status at rOpenSci Software Peer Review](https://badges.ropensci.org/521_status.svg)](https://github.com/ropensci/software-review/issues/521) Furthermore, if your package does not have a NEWS.md file yet, please create one to capture the changes made during the review process. See https://devguide.ropensci.org/releasing.html#news |
@ropensci-review-bot assign @vinhtantran as reviewer |
@vinhtantran added to the reviewers list. Review due date is 2022-04-20. Thanks @vinhtantran for accepting to review! Please refer to our reviewer guide. |
@vinhtantran: If you haven't done so, please fill this form for us to update our reviewers records. |
@ropensci-review-bot assign @wjones127 as reviewer |
@wjones127 added to the reviewers list. Review due date is 2022-04-29. Thanks @wjones127 for accepting to review! Please refer to our reviewer guide. |
@wjones127: If you haven't done so, please fill this form for us to update our reviewers records. |
Package ReviewPlease check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide
DocumentationThe package includes all the following forms of documentation:
Functionality
Estimated hours spent reviewing: 8 hours
Review CommentsI am not a Unity user or game developer, so I have to rely to the “Why do this?” section in the README to understand why the package is needed. The author pointed out two motivations: the use of closed-source tooling in current research and the potential of bias for personal preference. The first motivation is intriguing in that the package has the potential to encourage researchers to publish open-source and reproducible source codes for their studies. However, I hope the author can elaborate on the second point in terms of how the package would assist with the aforementioned issue. The source code was well-written and packaged. I have nothing to say about how the code was written and structured. It’s not surprising given that this is not the author’s first submission to rOpenSci. The vignettes are simple to follow and put into action. By following the 102 vignette, I was able to create a toy working example in Unity. I understand that the CRAN check and GitHub actions limit your ability to showcase more sophisticated examples with your assets, but I’d love to see one that highlights all package’s features, perhaps from a blogpost? It’s only a suggestion. Another point I’d like to make is about the package’s ongoing development. I cloned the package and wrote a bunch of comments on some inconsistencies in your function documentation a few weeks ago, only to discover that the majority of them have been fixed in your most recent commits, which were made a few days ago. Although the majority of them are minor changes, I would recommend that you include a note in the Review report to let us reviewers know which version we should work on. Finally, congratulations on a job well done. The review process taught me a lot, including new information about the R6 object in R. I’ve included some detailed comments below. Some of them may be obsolete in your most recent commit because I did not have time to revise them all. Detailed Comments
|
Thank you so much @vinhtantran ! I apologize for having you review a "moving target" -- I was doing a standard update across all my packages and completely forgot this one should be "frozen". I'll follow up with fixes and replies once our other review is in! |
I'm about half-way through my review, but in the meantime I've posted an issue in the repo: |
Package ReviewPlease check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide
DocumentationThe package includes all the following forms of documentation:
Functionality
Estimated hours spent reviewing:
Review CommentsOverall this package is very good. The code style and level of testing are excellent. I worked through the vignettes successfully, and then tried to create a simple scene of my own. I've reported the major issues I encountered in that process, which I think ought to be addressed (most already have 💯 ):
In addition, I noticed a few smaller issues while reviewing the package:
|
Will do! I've got a feature branch with changes in response to these reviews open at ropensci/unifir#5 , and I expect to have addressed all comments in the near future -- I'll update here once I've done so. Thanks everyone! |
Hi all! I believe I've addressed all reviewer comments in a branch on the unifir repo, https://github.com/mikemahoney218/unifir/tree/response_to_reviewers (diff at https://github.com/mikemahoney218/unifir/pull/5/files , NEWS at https://github.com/mikemahoney218/unifir/blob/response_to_reviewers/NEWS.md ). Thank you so much for your comments, the package is looking a lot better as a result! Response to @vinhtantranThanks so much for your review! I'd like to include you in the DESCRIPTION as a reviewer -- would you be able to tell me what I should list for "given" and "family" names? I don't want to infer incorrectly! Function documentation
Vignettes
Packaging guidelines
Response to @wjones127Thank you so much for your review! Below I'm only addressing the comments on this issue; I've dealt with the larger issues on their own tickets in the unifir repo. I'd like to include you in the DESCRIPTION as a reviewer, as well -- should I list you as Will Jones or something else?
|
Thanks for the response @mikemahoney218. @vinhtantran and @wjones127 can you please review the response and see if your comments were addressed? |
@mikemahoney218 I can confirm that all my comments have been addressed. I left a few optional follow-up comments in my final review of the changes here: ropensci/unifir#5 (review) Great work!
Yes, "Will Jones" is good to list me as. Email is willjones127 at gmail, if you need that. |
All of my comments have been addressed. Great work @mikemahoney218.
Please use "Tan Tran" with the email address of vinhtantran at GMail. |
Hello all @vinhtantran and @wjones127 thank you for the review! @mikemahoney218 since @wjones127 left a final review, please address that and let me know once it is done so I can approve.. |
@ropensci-review-bot submit review #521 (comment) time 8 |
Logged review for wjones127 (hours: 8) |
@ropensci-review-bot submit review #521 (comment) time 8 |
Logged review for vinhtantran (hours: 8) |
Hi @melvidoni ! All of the last changes have been made (all resolved as part of ropensci/unifir#5 (review) and ropensci/unifir#7 ) so I believe we're all set here. Thanks! |
@ropensci-review-bot approve unifir |
Approved! Thanks @mikemahoney218 for submitting and @vinhtantran, @wjones127 for your reviews! 😁 To-dos:
Should you want to acknowledge your reviewers in your package DESCRIPTION, you can do so by making them Welcome aboard! We'd love to host a post about your package - either a short introduction to it with an example for a technical audience or a longer post with some narrative about its development or something you learned, and an example of its use for a broader readership. If you are interested, consult the blog guide, and tag @ropensci/blog-editors in your reply. She will get in touch about timing and can answer any questions. We maintain an online book with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding (with advice on releases, package marketing, GitHub grooming); the guide also feature CRAN gotchas. Please tell us what could be improved. Last but not least, you can volunteer as a reviewer via filling a short form. |
@ropensci-review-bot finalize transfer of unifir |
Transfer completed. The |
@melvidoni I'm not sure if there's an expected lag, but the bot doesn't appear to have added me to the unifir team (and as such I don't yet have admin on the repo) |
Hello @mikemahoney218. You'll have to wait so I can ask about the bot. I believe it should have created the group. |
No worries! It did create the team -- https://github.com/orgs/ropensci/teams/unifir -- but didn't add me. I'm on the terrainr team ( https://github.com/orgs/ropensci/teams/terrainr ) so I don't believe there's any issue with my MFA setup. No rush at all! |
I had to approve you, weird, I'm exploring. In any case you've now been added to the team @mikemahoney218! |
Awesome, thanks @maelle ! In case it helps debugging, I manually requested access to the team after the bot didn't add me, I didn't get added without approval. I believe that means the repo is transferred and we're all good to go! Thank you so much everyone! |
You might be the first "repeat" submitter since we started using the bot, we'll investigate. Thanks for catching this bug and your patience 😁 |
Date accepted: 2022-05-01
Submitting Author Name: Mike Mahoney
Submitting Author Github Handle: @mikemahoney218
Repository: https://github.com/mikemahoney218/unifir
Version submitted: 0.1.0
Submission type: Standard
Editor: @melvidoni
Reviewers: @vinhtantran, @wjones127
Due date for @vinhtantran: 2022-04-20
Due date for @wjones127: 2022-04-29
Archive: TBD
Version accepted: TBD
Language: en
Scope
Please indicate which category or categories from our package fit policies this package falls under: (Please check an appropriate box below. If you are unsure, we suggest you make a pre-submission inquiry.):
Explain how and why the package falls under these categories (briefly, 1-2 sentences):
Following discussion in Pre-submission Inquiry: unifir, A Unifying API for Working with Unity in R #508, I believe this package is within scope for the following reason:
This package provides bindings to the Unity video game engine API from R, for the production of 3D/VR "environments" entirely from R code. It enables producing a new type of visualization in an efficient and reproducible manner, and provides specific methods for visualizing spatial data stored in standard spatial formats. However, I could understand if the package itself is too general for rOpenSci in particular; while we are designing it for research applications, Unity is not explicitly scientific software.
The target audience is anyone looking to produce data-driven immersive virtual environments in Unity, with a primary focus on visualizing natural environments. I work directly with ecologists and landscape architects, and so the initial feature set is oriented towards making the package useful for that audience.
Immersive virtual environments are an active area of research (see for instance https://www.mm218.dev/papers/vrs_2021.pdf, https://joss.theoj.org/papers/10.21105/joss.04060, https://link.springer.com/article/10.1007/s42489-020-00069-6 ). At the moment, the current standard for the field is producing hand-designed "artistic" environments tailored for each purpose, which makes assessing this visualization method difficult. Our aim with this project is to produce a standard set of tooling for creating immersive virtual environments, making it both easier to produce visualization "bases" for applications and giving us a way to produce reproducible visualizations as a baseline for assessing visualization effectiveness directly.
I'm not aware of any. The closest is likely the rayverse (rayshader/rayrender), which makes fantastic 3D visualizations in R directly, without incorporating Unity; the incorporation of Unity makes adding player controllers (for interactive exploration of the environment) much easier.
N/A
#508
pkgcheck
items which your package is unable to pass.x These functions do not have examples: [check_debug, create_if_not].
These functions are not exported, which given my past experiences with CRAN implies that they should not have examples (as apparently examples are run for non-exported functions on test machines).
Technical checks
Confirm each of the following by checking the box.
This package:
Publication options
Do you intend for this package to go on CRAN?
Do you intend for this package to go on Bioconductor?
Do you wish to submit an Applications Article about your package to Methods in Ecology and Evolution? If so:
MEE Options
Code of conduct
The text was updated successfully, but these errors were encountered: