Feature/uk pp te merger - #2144
Conversation
lukavdplas
left a comment
lukavdplas
left a comment
There was a problem hiding this comment.
Looks good, just some comments about naming. (And the unit tests are failing.)
| @@ -0,0 +1,6 @@ | |||
| The debates of the two chambers of the British bicameral Parliament, the House of Lords and the House of Commons, based on Hansard, collected and enriched by two research teams: the People and Parliament research group (University of Jyväskylä) and the Talking Empire research group (University of Southern Denmark). | |||
There was a problem hiding this comment.
"Hansard" may not be self-explanatory. Maybe add a a link here.
| title = 'UK Parliament (P&P + TE)' | ||
| description = "Speeches from the House of Lords and House of Commons" |
There was a problem hiding this comment.
Most visitors will not know what P&P or TE stand for. I would just call the corpus "UK Parliamentary Debates" or something. You can mention the projects in the description?
| description = "Speeches from the House of Lords and House of Commons" | ||
| min_date = datetime(year=1803, month=1, day=1) | ||
| max_date = datetime(year=2025, month=12, day=31) | ||
| es_index = 'parliament-uk-merged' |
There was a problem hiding this comment.
Why not just parliament-uk?
There was a problem hiding this comment.
I chose to pick a new index name partly because this merged corpus will get one more round of revision and an added field while the researchers are already using the first version, and I did not want to mess with any existing workflow in People and Parliament, thinking that we could 'fully migrate' to the new merged corpus later. The new merged index is backward-compatible with the old P&P filters, so old URLs would still work I think.
There was a problem hiding this comment.
Fair enough. This would not affect URLs in the frontend though, users can't see the index name.
|
|
||
| image = 'uk.jpeg' | ||
| languages = ['en'] | ||
| description_page = 'uk-merged.md' |
There was a problem hiding this comment.
Same here, just use uk.md?
| docs = get_documents(corpus, start, end) | ||
| for target in corpus_object.get('docs'): | ||
| doc = next(docs) | ||
| print(doc) |
lukavdplas
left a comment
lukavdplas
left a comment
There was a problem hiding this comment.
Not sure if you were waiting for another review? But it looks good to me 👍
2 participants
This branch contains updated fields for the P&P UK corpus, as well as a merged corpus definition that includes the more recent Talking Empire data.