Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

Description and citation as markdown #190

Merged
merged 12 commits into from
Jan 29, 2021
Merged

Description and citation as markdown #190

merged 12 commits into from
Jan 29, 2021

Conversation

AkshitaB
Copy link
Contributor

@AkshitaB AkshitaB commented Jan 8, 2021 •
edited
Loading

Fixes allenai/allennlp-demo#705

Edit: requires merging allenai/allennlp#4919

@AkshitaB AkshitaB requested a review from epwalsh January 15, 2021 17:23
jonborchardt
jonborchardt reviewed Jan 15, 2021
View reviewed changes
Copy link

@jonborchardt jonborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left some thoughts

allennlp_models/modelcards/coref-spanbert.json Outdated Show resolved Hide resolved
allennlp_models/modelcards/generation-bart.json Outdated Show resolved Hide resolved
allennlp_models/modelcards/lm-masked-language-model.json Show resolved Hide resolved
jonborchardt
jonborchardt reviewed Jan 20, 2021
View reviewed changes
Copy link

@jonborchardt jonborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left some comments

allennlp_models/modelcards/generation-bart.json Outdated Show resolved Hide resolved
allennlp_models/modelcards/pair-classification-decomposable-attention-elmo.json Outdated Show resolved Hide resolved
allennlp_models/modelcards/coref-spanbert.json Outdated Show resolved Hide resolved
jonborchardt
jonborchardt approved these changes Jan 20, 2021
View reviewed changes
Copy link

@jonborchardt jonborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lg2m,
1- can we fix the caveats_and_recommendations issue
2- merging thius will break prod until the model card ui is fixed.
https:/allenai/allennlp-internal/issues/150

one option would be to unpublish the new reading-comp card until this work is done

@AkshitaB
Copy link
Contributor Author

@jonborchardt

lg2m,
1- can we fix the caveats_and_recommendations issue
2- merging thius will break prod until the model card ui is fixed.
allenai/allennlp-internal#150

one option would be to unpublish the new reading-comp card until this work is done

1 - I would like to keep it as it is. Right now, it's a single key in the object, but I would like there to be flexibility to add related keys.
2 - Yes, it also requires this PR to be merged - allenai/allennlp#4919 Can you review that as well? (The checks fail on both PRs because they are dependent on each other).

@jonborchardt
Copy link

can we just rename the outer caveats_and_recommendations then?

@AkshitaB
Copy link
Contributor Author

@jonborchardt

can we just rename the outer caveats_and_recommendations then?

Done.

@AkshitaB AkshitaB merged commit 42c0450 into main Jan 29, 2021
@AkshitaB AkshitaB deleted the update-model-cards branch January 29, 2021 00:21
AkshitaB added a commit that referenced this pull request Jan 29, 2021
@AkshitaB
Description and citation as markdown (#190)
0413a7e 
* description and citation as markdown

* fixing papers and datasets

* fixing formatting

* fixing keys and citation

* remove relative url

* rename keys

* bug fix
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jonborchardt jonborchardt jonborchardt approved these changes

@epwalsh epwalsh Awaiting requested review from epwalsh

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

model cards should use markdown
2 participants
@AkshitaB @jonborchardt