Closed olendorf closed 6 years ago
[x] Files open as expected
[x] Code Runs as Expected
[x] Metadata is accurate and complete
[x] Documentation
[x] README
[x] Codebook / Data Dictionary
all files open as expected
added main.R
to automate the running of the 4 steps. Also edited STEP_1_v20_pers_Sample.R
to automatically download external files so the user doesn't have too.
Metadata appears to be excellent.
Excellent documentation, found no obvious errors and was able to run and update code easily.
[x] Data well structured
[x] Headers defined
[x] Codes defined
[x] Blanks and missing values defined
[ ] Quality control well defined.
I don't see quality control explained, but data appears internally consistent. Also software dependencies are very well explained. I did get some warning running the code, but my R is a bit out of date so that could be it.
There are small things that can be done. I am suggesting some additions and changes to the R scripts, and changes to the README to reflect those changes. The changes are in a pull request but listed here
main.R
to run all the scripts at onceSTEP_1_v20_pers_Sample.R
to automate downloading of external datastep_one.R
, step_two.R
. The remaining
words seem unnecessary.[x] Discoverability sufficient
[x] Keywords sufficient
[x] Linkage Sufficient
The README provides considerable information to aide discoverability on the web. The manuscript is still in prep so the link should be made when appropriate. It is the repositories responsibility to assign a DOI or other suitable PIDs.
[x] Preferred file formats used
[x] Software is readily available
[x] Visualization is readily accessible
[ ] Software is easily run
The file formats are all text, CSV or TIFF which are all best practice formats. R and the associated packages are all open source and easily obtained and run. Additionally the versions under which the analysis was run are given (good job).
The only issue I have is downloading the external data, and running each script is a bit tedious and error prone. I have modified the scripts to fix this and proposed it in a pull request.
[x] Metadata exceeds author/title/date
[ ] Unique PID
[x] Discoverable vis web search
[ ] Retrievable via standard protocols
[ ] Free / open
[ ] Metadata uses standard schema
[ ] Metadata in machine readable format
[x] Data include sufficient metadata to allow reuse
[x] Contact info easily found
[x] Clearly defined ownership and stewardship
[x] Appropriate open licensing
As much as possible all of the components for finding, access, and reuse are there. Interoperability is really the responsibility of the home repository so I can't evaluate that. Likewise, accessibility in the sense of PIDs and supported protocols are the responsibility of the repo. The licensing however, is very good and the creator does an excellent job describing how to be "nice" too.
I believe this submission is good to go, but if the creator accepts the pull request #3 then it will be further enhanced.
@dangercat you should send heidi an email now.
@olendorf - wow, you've got some savvy stuff in here I don't think I know about (like the download directly stuff). Thank you!!! Lots of good suggestions. Give me a little bit to looks at the suggestions and make sure I understand/they do what I think is right. That will take me all week probably, I'm in the middle of something else. I'm also new to github so I might have some questions about pull requests (sorry about that! - I guess, if someone doesn't know the platform well, some of the "curation" might be user help instead ... I wonder if that's okay...? - good lesson to learn now)
FYI for @dangercat
Oops - accidentally put this in the pull request instead of the issue - probably best here... @olendorf @dangercat - looked at everything and questions posed in the pull request. back to Rob! Thanks!
@1heidi @dangercat I think at this point its up to Liza or hiedi to merge if it looks ok.
Sounds right to me.
On Sat, Aug 25, 2018 at 9:13 AM Liza Coburn notifications@github.com wrote:
If Heidi is happy with everything I think it’s up to her to merge the changes she’s accepting and let us know she’s signing off (and if we were using Jira right now she’d mark the issue as resolved and I guess I’d review and close it)...?
On Aug 25, 2018, at 8:04 AM, Robert Olendorf notifications@github.com wrote:
@1heidi @dangercat I think at this point its up to Liza or hiedi to merge if it looks ok.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/1heidi/nar_persistent/issues/1#issuecomment-415968452, or mute the thread https://github.com/notifications/unsubscribe-auth/AA0Wp0kRG26wDmHLRtw1E33Le_NWq7YJks5uUU2NgaJpZM4VymPE .
I have merged. I think the DCN part of this is done (I just need to do some things as the depositor). Thanks!!
This is just to track the status