[REVIEW] for Joss

[vsoch]

Review

This is software that can calculate coordinates based on addresses, provided in a container for reproducible use. It's well documented and described. My main comments have to do with clearing up some of the examples and description. I understand the use of containers for reproducibility, but it's not clear to me how geocoding an address (that can be reversed) somehow ensures privacy. Please read my comments below for details about how it might be better described (in human friendly terms for the layperson) and thank you for the opportunity to review this work. [Accept]

Criteria

Does the paper ONLY include these things?

• [x] A list of the authors of the software
  • confirmed that the main author is the only contributor to the repository
• [x] Author affiliations
• [x] A summary describing the high-level functionality and purpose of the software for a diverse, non-specialist audience. It may not be clear to some what geocoding is to begin with. There was a popular thing called "geocaching" which was hiding things at particular coordinates years back, and it might be confused with that (or generally other "geo" terms). A simple sentence like "Geocoding is the practice of..." would be sufficient for the layperson. It's also not clear to the layperson what a "container" is and how using one addresses some of the other issues. I can read this ok coming from a container + science background, but if not? I would have trouble.

• [x] A clear statement of need that illustrates the purpose of the software

  • the statement is there (the first paragraph) but it is more "academically written" than "human friendly" and concise. You really just need to say "Geocoding might capture someone's private information, like where they live, and this is against HIPAA, etc.). A shorter, more concise statement that gets the point across (with references to more verbose detail) would be more appropriate here. I can offer to help, it seems like what you want to say is:
  1. Location-based data is important for different kinds of research
  2. It would go against rules to publish someone's personal location information
  3. We are missing standard ways to make this easy.

General Review of Paper

Here are a few things I genuinely don't understand / know what they are based on not being a part of the field:

• "de-rive community and environmental exposures" (you mean like exposure to car exhaust? violence?)
• "that can produce geocodes" (what does this mean it is doing? Does someone carry around a computer through a neighborhood and record things?)
  • "both indepen-dently geocode their own addresses and link in the necessary place-based characteristics" (I don't know what this means, maybe you would instead of stating this, walk the reader through an actual example?)
• "sends de-identified dataset out for analysis" (how?)

Ah, here is where you start to explain it well:

It was designed to be used by scientific researchers who wish to collect place-based data on study subjects and patients with a residential address.

The figure is nice, but a caption that more clearly explains it would be great!

• [x] Mentions (if applicable) of any ongoing research projects using the software or recent scholarly publications enabled by it
• [x] A list of key references including a link to the software archive
• [x] confirmed that the paper doesn't have usage, API docs, etc.

Review items

Software

• [x] Versions: The tag on Github (v0.2) is different than the Docker image tags (v2.2). Could the author please comment on this discrepancy, and consider maintaining consist versions as to not be confusing for the user?

  ---

  Update September 10, 2018 The versions are coinciding with tags in a different repository, via an automated docker build. This is okay.

  ---

Software license

• [x] OSI approved, text file in respository.

Testing the software, I get these warning messages:

Loading required package: methods
  |============================================================| 100%, ETA 00:00
Warning messages:
1: CBapply is deprecated but is maintained here for backwards compatability.
Consider using CB::mappp instead. 
2: `as_function()` is deprecated; please use `as_mapper()` or `rlang::as_function()` instead 

---

Update September 10, 2018 This warning message has been suppressed.

---

Given the container, has the author considered following the suggestion to eliminate the warning?

Documentation

Here I am reviewing the README.md in the main repository.

• [x] A statement of need
• [x] The authors should clearly state what problems the software is designed to solve and who the target audience is. _Would the author consider moving the "About" to be closer to the top, so it's seen first?"
• [x] There should be a clearly-stated list of dependencies. Ideally these should be handled with an automated package management solution. Very well done, containers, and great documentation for installation, etc.!
• [x] Example usage. This isn't addressed directly, but would be easy to do! How about a list of bullet point examples for how this might be used? This would help me to understand better too :).
• [x] The authors should include examples of how to use the software (ideally to solve real-world analysis problems). The examples aren't provided, but the actual walk through of the example input file and using the container is spot on!

---

Update September 10, 2018 The about section and example usage are fabulous.

---

One quick suggestion - the author provides a sample file with the column name address and then the example command is:

docker run --rm=TRUE -v "$PWD":/tmp degauss/geocoder my_address_file.csv addresses

I think it would make sense to 1) tell the user to name the file "my_address_file.csv" and then have the same column name (one or the other) so literally the user can copy paste the command (without change) from the $PWD with the file and it will work.

What isn't clear from this example is what I just did. I converted a file of addresses to... another file? Maybe describing this in the context of an actual example (e.g., "Let's say you have these addresses that represent... and you want to..." here is how you would do this.).

---

Update September 10, 2018 This file naming has also been updated and fixed.

---

• [x] API documentation is good.
• [x] For the next example, I don't know what geomarker assessment is (when you have latitute and longitude?) Perhaps each of these examples in the walk through would be best paired with a description of a real world example for noobs like me. ;) It might also make sense to continue the walkthrough from the previous step, first give the general instruction, and then show with the file that was just produced, e.g.:

docker run --rm=TRUE -v "$PWD":/tmp degauss/dist_to_major_roadway <name-of-geocoded-file>
docker run --rm=TRUE -v "$PWD":/tmp degauss/dist_to_major_roadway my_address_file_geocoded.csv

• [x] Missing values - why would a value be missing, and is there some sort of partial information the viewer could return (in a separate file?) to investigate or debug something?
• [x] Question - if these two containers can reverse one another (e.g., I produce the geocoded file and then calculate a distance a major roadway) why are they maintained in separated containers? Has the author considered providing the software in a single container (to keep the versions consistent for the user - I won't download and use different ones) and then provide multiple entrypoints? Are the dependencies quite different?

References

There are a bunch of empty lines in the references (maybe authors?) that might be looked at.

Community guidelines

• [x] Community guidelines are missing. The author has a clear link to getting help, but could you please add a CONTRIBUTING.md with instructions for how to contribute?
• [x] Functionally, I tested the examples in the README and it works as expected. Is the author intending for more advanced users to ever shell into the image and interact directly with the software?
• [x] There are no tests. Has the author considered setting up continuous integration?

[vsoch]

And by the way, this is really cool! Given standard inputs and outputs, a lot of cool visualization tools could be built around this!

[cole-brokamp]

Hi @vsoch, I just wanted to stop by and say thank you for the valuable feedback and review. I just moved to a new house and am up against a couple of academic deadlines :weary:, but I intend on updating the software according to your input as soon as I can. Thanks again!

[vsoch]

Congratulations on your move!! No worries, please take the time you need. I'm in the middle of moving too (the truck is still... who knows where...) and I can completely empathize. I'm mostly avoiding thinking about the "I need to unpack when things finally arrive" part crawls into fetal position and thinks of someday-when-I-am-not-moving

[cole-brokamp]

I've added a commit that has addressed all of your comments, @vsoch. Please review and let me know what you think when you have time. Thanks again for the helpful and constructive feedback! I'm happy to see DeGAUSS improving as a result of the peer review process. I've outlined my changes below, roughly in order of your review above:

Changes Made for JOSS Review

Added summary describing the high-level functionality and purpose of the software for a diverse, non-specialist audience

• defined "geocoding"
• defined "container" and how it addresses some of the scientific issues

Added clear statement of need that illustrates the purpose of the software

• made need statement more "human friendly" and concise

Add clarifications to paper

• defined and added examples of community and environmental exposures
• removed the confusing term "produce geocodes"
• referred the reader to the README for an example, including how to share deidentified data
• added better caption to the figure

Sofware Changes

• The GitHub tags do indeed match the DockerHub tags -- its just that DeGAUSS/geocoder is hosted in a separate GitHub repo. This provides separation between "geocoding" and "geomarker assessment". The matched tags are achieved through automated builds on DockerHub triggered by a new tagged release on GitHub.
• suppressed warning messages for usage of CB::CBapply

Documentation

• Moved "About" section to top so it is seen first
• Made sure that the problems that the software can solve are more clearly stated and identified who the target audience is
• Added a list of specific example usages in the "About" section
• Adjusted code example so user can copy/paste if using file named my_address_file.csv with address column
• Added output of example usage and extended it to geomarker assessment
• Added why some geocodes might be missing and how users can troubleshoot this problem.
• I have chosen to keep DeGAUSS tools in separate containers because their dependencies are quite different and DeGAUSS is designed to be extendable by the user for other geomarker datasources and assessment toolkits. The geospatial datasets used are commonly > 1 GB and keeping each geomarker in a separate container not only reduces the image size, but more importantly, keeps the entire system modular and more reproducible.

References

• The references missing authors are referring to legislation, which is commonly cited without an author given the understanding that it is "United States Public Law"

Community Guidelines

• Add community guidelines with a CONTRIBUTING.md
• I do not expect advanced users to shell into the image and interact directly with the software. DeGAUSS is designed specifically for users that want to geocode and do geomarker assessment, but do not have the technical expertise to use the underlying software. If a user is familiar and capable with these processes, then the only advantage of DeGAUSS would be its reproducibility -- it would likely prevent more of an obstacle than an assistance. Although I have left this out in the README, I often use this technique to troubleshoot with others remotely and will add it to the wiki page.

[vsoch]

Awesome! Will have some time to look later today, stay tuned!

[vsoch]

Review after changes

Hey @cole-brokamp ! Apologies for the delay, my state (NC) is preparing for a storm of doom and everyone is freaking out, myself included because I evacuated my apartment this morning after the predicted storm category increased and debating for a bit. Now I'm at least somewhere with a generator, and will be assured internet connectivity! I can proceed with the review!

• The README.md and abstract are so much better! As someone not familiar with the domain you teach me everything that is important to know about the software - both from a rationale for "Why are you doing this period for science" to "How is this tool important for that goal?" The description of containers is also just at the right level. You can only take on so much education for every little piece of technology, and it's up to the reader to do some google searching in the case of wanting more information.
• I see the matching versions now, thank you for pointing me to that. +1 on the automated builds! I like this feature too.
• All of my needs are addressed here, and great job with the community guidelines and contributing docs.

You have responded very well to all of my points, and I would suggest this paper for publication. I will go back to the original thread to look over the paper once more.

[cole-brokamp]

Thank you for the quick followup and I hope that you are staying safe!! I had one question about implementing tests. This is something that I want to do, but am struggling with the specifics. Do you think it would be wise to implement some formal unit tests? Or are you thinking more along the lines of "Here is a sample address file and if you run it through these three containers using this specific code, then here is what the output should look like at each step"? Does that make sense? I appreciate any insight you might have.

[vsoch]

hey @cole-brokamp! You would minimally want those functions run against the data files on each commit to master, using a service like Travis CI or Circle CI. It’s something you don’t have to think about, and any user can run the equivalent tests manually. I can offer to create a simple setup for you (and issue a PR, you would need to officially connect the service from your web interface) to help you learn by just seeing what I mean. What do you think?

[cole-brokamp]

I see. I am familiar with tests in R, but have no experience creating test for bash commands. I don't want to create any additional work for you -- can you point me in a helpful direction and I can ask you for more help if I need it?

[vsoch]

Sure! Here you go https://circleci.com/docs/2.0/getting-started/ what you can maximally do is set up a build test and deploy cycle where the containers will be built and deployed to Docker Hub after passing the tests. But if you prefer to just push manually this is overkill, and you can just have a simple setup that builds the containers and runs the same commands the user would. You basically are going to write a .circleci/config.yml file that will issue these commands to test! Yaml and their workflows are a bit funky so please ask me if you need help. I’ve made these so many times I can do it pretty quickly. Good luck!

[cole-brokamp]

thanks! I will dive in this weekend and let you know how it goes

[cole-brokamp]

I (finally!) got time to add tests through Travis. Each of the three images is built and a command is ran on each using a sample geocoded address file and Travis checks to make sure the expected output file is present. Thanks for your help in teaching me about this!

[vsoch]

Fantastic work, and your tests just pass! This definitely meets all the criteria for publication, and after the fact when you feel up for it, you can give a go at adding a deploy step so in the future the docker hub builds are just magically done for you (tagged with either a software version or Github commit). I'll update the main thread to let the editors know! I'm going to close here.