Making the Package Public

[BatoolMM]

This PR aims to address issue #1

Heads-up, I am not expecting this PR to be merged at all (just realised after I finished it) and it doesn't address some of the issue. My intention is to share questions/points from an external user's perspective, highlighting potential challenges in using or contributing to the package, and suggesting changes for user-friendliness. I also wanted to verify certain design choices in the package structure to ensure that my modifications align well with your original vision.

What I am trying to do?

• Reviewing the DESCRIPTION File: I encountered an error linked to the Maintainer field in the DESCRIPTION file. I've adjusted it to include an email address, as per standard practices, hope that's OK.

• License Addition: I've added a GPL license, but this can be substituted with a permissive license like MIT license, especially considering the common use of MIT in scientific packages. I'm open to modifying this as per your preference.

• Package Review and CMD Check: This is just a routine I do with any package I am building or reviewing to double check if it passes all checks. I am having a few warning with this package:

W  checking dependencies in R code ...
   'library' or 'require' calls not declared from:
     ‘grid’ ‘gridExtra’ ‘insight’ ‘rjson’
   'library' or 'require' calls in package code:
     ‘grid’ ‘gridExtra’ ‘insight’ ‘rjson’
     Please use :: or requireNamespace() instead.
     See section 'Suggested packages' in the 'Writing R Extensions

N  checking R code for possible problems (4.7s)
   domain_mapping: no visible global function definition for ‘data’
   domain_mapping: no visible binding for global variable ‘json_metdata’
   domain_mapping: no visible binding for global variable ‘domains_list’
   domain_mapping: no visible global function definition for ‘fromJSON’
   domain_mapping: no visible global function definition for ‘read.csv’
   domain_mapping: no visible global function definition for ‘plot.new’
   domain_mapping: no visible global function definition for ‘grid.table’
   domain_mapping: no visible global function definition for
     ‘print_colour’
   domain_mapping: no visible global function definition for ‘write.csv’
   Undefined global functions or variables:
     data domains_list fromJSON grid.table json_metdata plot.new
     print_colour read.csv write.csv
   Consider adding
     importFrom("graphics", "plot.new")
     importFrom("utils", "data", "read.csv", "write.csv")
   to your NAMESPACE file.

W  checking for missing documentation entries ...
   Undocumented code objects:
     ‘domains_list’ ‘json_metdata’
   Undocumented data sets:
     ‘domains_list’ ‘json_metdata’
   All user-level objects in a package should have documentation entries.
   See chapter ‘Writing R documentation files’ in the ‘Writing R
   Extensions’ manual.    

I was going to go through them but I thought I would ask you first if there is a specific reason why you would prefer to keep any of them. If not, I am happy to go and address them.

• Making the Package Citable: It would be great to facilitate citation of this package. This can be achieved through various means like adding a CFF file, notes in README, or linking the repo with Zenodo, all can be done at the end unless you prefer otherwise.

• Make documentation easier to navigate by adding a vignette and pkgdown website: In my experience, adding a vignette and a pkgdown website often enhances a package's usability. Were these omissions intentional, or would you be open to me working on them? Perhaps it would be best addressed in a separate PR. I've included an example in this PR for your reference.

• Make installation of the package easier: I'm interested in simplifying the installation process for the package. I noticed that there's an Rproj file nested within another Rproj, with the package located in the second Rproj. This setup might be confusing for contributors, especially if multiple people are to maintain this package post-RSF project. Could you clarify the reasoning behind this structure? It might be beneficial to outline this in the README or consider relocating all package files to the root directory to maintain a single Rproj for simplicity.

• Adding renv to the package: Adding renv for dependency management is on my radar, likely as a part of a subsequent PR.

Increasing Package Visibility:

Finally, I believe this package has great potential for wider use and recognition. Submitting it to rOpenSci and featuring it in the RWeekly newsletter once it's public could be beneficial, providing that you're comfortable with that. Maybe we can discuss it offline or in another GH issue.

VERY sorry for the lengthy message and late PR. I'm happy to close it based on your feedback and clarifications.

[BatoolMM]

Also, if there is anything I missed and you wanted me to focus on, give me a shout!

[RayStick]

Thanks @BatoolMM - this is all helpful. Just to double check for my own understanding - did you manage to successfully run the function _domainmapping?

[BatoolMM]

Just to double check for my own understanding - did you manage to successfully run the function _domainmapping?

No, this one didn't work!

[RayStick]

No, this one didn't work!

Ah, interesting. I think Mahwish got it to work when testing it It would be good for me to understand why - before implementing some of these larger changes. Could we have a share-screen call sometime? That would be the easiest way to troubleshoot and see how it looks different on your computer versus mine I think!

[RayStick]

First step

Can you put in a new PR in for Reviewing the DESCRIPTION File and License Addition - these are simple steps that we could get merged in soon. I could cherry-pick your commits for these changes, but a new PR may be simpler?

Also, I am more familiar with MIT license - can you remind me of the differences between MIT and GPL? Then we can make a final decision on what license to include.

Next steps

• I'd like to understand more about the details of this check - Package Review and CMD Check:. But first it would be good to see if we can get the function working together on your computer via a share screen.

• Making the Package Citable: Happy with adding license file and 'how to cite' info to the README for now, and keep the other suggestions for later. Maybe group the remaining suggestions with the points under 'Increasing Package Visibility:' and make an Issue about this once the repo is public?

• Make documentation easier to navigate by adding a vignette and pkgdown website: Great ideas. I think I see this as 'extras' and so we could work on this after package is public? Thanks for the examples, I will take a look.

• Make installation of the package easier: I am not sure I fully follow so would be good to discuss in more detail. The repo name is 'browse-SAIL' and the package folder name is 'browseSAIL' so I think that is a bit confusing! On reflection, I might want to come up with some better names for the repo and package and function.

• Adding renv to the package: Yeah I think we leave this for now, and return to it later as an enhancement.

[RayStick]

Thanks for all your work on this =D Please see my two comments above!

[BatoolMM]

Of course, I will add a new PR for Reviewing the DESCRIPTION File and License Addition - no need to cherry-pick.

In terms of the license:

The major difference between the two is that GPL means that anything built on the top of this code should also be open so it make it tricky to commercialise a product that one of it's dependencies use a code licensed with GPL. This is like how Linux is licensed or how I was taught.

[RayStick]

Thanks for clarifying! Happy with GPL. Can we change it later if there was a reason to?

[BatoolMM]

Thanks for clarifying! Happy with GPL. Can we change it later if there was a reason to?

I am not a lawyer but I don’t think we are encouraged to change license midway. Maybe let's use MIT to make things easier for research consortia.

[BatoolMM]

First step

Can you put in a new PR in for Reviewing the DESCRIPTION File and License Addition - these are simple steps that we could get merged in soon. I could cherry-pick your commits for these changes, but a new PR may be simpler?

Also, I am more familiar with MIT license - can you remind me of the differences between MIT and GPL? Then we can make a final decision on what license to include.

Next steps

• I'd like to understand more about the details of this check - Package Review and CMD Check:. But first it would be good to see if we can get the function working together on your computer via a share screen.
• Making the Package Citable: Happy with adding license file and 'how to cite' info to the README for now, and keep the other suggestions for later. Maybe group the remaining suggestions with the points under 'Increasing Package Visibility:' and make an Issue about this once the repo is public?
• Make documentation easier to navigate by adding a vignette and pkgdown website: Great ideas. I think I see this as 'extras' and so we could work on this after package is public? Thanks for the examples, I will take a look.
• Make installation of the package easier: I am not sure I fully follow so would be good to discuss in more detail. The repo name is 'browse-SAIL' and the package folder name is 'browseSAIL' so I think that is a bit confusing! On reflection, I might want to come up with some better names for the repo and package and function.
• Adding renv to the package: Yeah I think we leave this for now, and return to it later as an enhancement.

Just a heads-up, I will open issues for each of these steps so we can close this PR and fix each issue separately.

[BatoolMM]

I think this PR can be closed - new issues were created #26 #34 #33 #35 to address all points you mentioned!

[RayStick]

I think this PR can be closed - new issues were created https://github.com/aim-rsf/browse-metadata/issues/26 https://github.com/aim-rsf/browse-metadata/issues/34 https://github.com/aim-rsf/browse-metadata/issues/33 https://github.com/aim-rsf/browse-metadata/pull/35 to address all points you mentioned!

Sounds good to me