Skip to content
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

Review of submission 17tizard #18

Closed
neilernst opened this issue Jul 3, 2019 · 8 comments

Comments

@neilernst
Copy link
Collaborator

commented Jul 3, 2019

No description provided.

@timm

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

@jtiz003 : I take it the install instructions for this are

  • install sqlite
  • then have fund browsing around the database

please confirm.

@BuddyWasisname2019

This comment has been minimized.

Copy link
Collaborator

commented Jul 8, 2019

The data is as described with the attached README.md, containing three tables (POS, labelled_sentences, verbs). The description of table labelled_sentences is consistent with the actual dataset. However, the POS and verbs table are lacking documentation (they contain more than simply an ID column), especially with respect to column names, which are missing in the README.md. In addition, it might be beneficial to provide a more verbose name for the POS table. Of course, some of these details might be covered in the paper itself (of which I do not yet have access to).

In addition, perhaps an entity relationship diagram would help with the documentation of the tables, since they are relational -- this should include the primary/foreign keys for each table.

Overall, the data appear to be interesting; although the data could benefit from being normalized (industry standard for relational databases); mainly for consistency:
eg:
dispraise application
vs
dispraise for application
and
problem resolution
vs
Problem resolution

Finally, I do not think the availability aspect of the the artifacts is met, as it is only available on THIS GitHub repository (as opposed to something like zenodo). So if the authors upload the artifacts to a publicly available location then perhaps we can consider upgrading the badge to available.

For now, provided that the documentation is updated to be "very carefully documented" as per my aforementioned comments, I recommend the badge of reusable.

@neilernst

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 12, 2019

Hi @jtiz003 - we will be finalizing badges this weekend - can you please respond to the review?

@RESubmission17

This comment has been minimized.

Copy link

commented Jul 13, 2019

Sorry for the slow reply.

I have updated the readme on the original repo with the following additions
(https://github.com/RESubmission17/RE_Submission_17/blob/master/README.md)

  • The POS and verb table documentation has been updated with complete descriptions. Hopefully this will make their content clearer

  • Access/install instructions have been added

We have made the data available on Zenodo with a link provided in the paper (https://zenodo.org/record/3315707#%23.XSZ8_-gzZPYz)
The readme hasn't been updated on Zenodo yet, but I will do this once the documentation is finalized.

Feedback classifications: We propose a new set of classification in our paper, so the data follows these.

Please let me know if anything additional is needed.

@neilernst @BuddyWasisname2019 @timm

@timm

This comment has been minimized.

Copy link
Contributor

commented Jul 14, 2019

@BuddyWasisname2019 : please note our current decision is reusbale unless you elect to fault the revised doc. nbow no doco is perfect but is the new doco better?

@jtiz003

This comment has been minimized.

Copy link
Collaborator

commented Jul 14, 2019

@timm @neilernst Are we missing something for available? The data is available on zenodo

@BuddyWasisname2019

This comment has been minimized.

Copy link
Collaborator

commented Jul 14, 2019

Based on the updated documentation and the added Zenodo link, I recommend the badge of available.

@timm

This comment has been minimized.

Copy link
Contributor

commented Jul 14, 2019

ok, @neilernst : I think we should rebadge this "available" (not reusable). do u concur?

@timm timm closed this Jul 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.