Feature/hansard 2026#2077
Conversation
…gitalHumanities/I-analyzer into feature/hansard-2026
JeltevanBoheemen
left a comment
JeltevanBoheemen
left a comment
There was a problem hiding this comment.
Well done, seems like quite smoe work to figure out the data cleaning.
Remark on the corpus/file name: using new to indicate a version is not future proof. What happens when we get another update? We will end up with things like Index_Current_New and Index_Current_Old (actual example from another project). Maybe call it uk_hansard?
Some nitpicky code quality comments, feel free to ignore if you consider them too small to fix :)
|
|
||
|
|
||
|
|
||
| def extract_date(path: str): |
There was a problem hiding this comment.
| def extract_date(path: str): | |
| def extract_date(path: str) -> Optional[str]: |
Type annotations are not consistent in this file. Not a big deal, but if you're doing them anyway, might as well make them complete.
Annotating all parameters types has higher prio then return types imho.
| if 'daylord' in path: | ||
| return 'House of Lords' | ||
| elif 'debates' in path: | ||
| return 'House of Commons' | ||
| else: | ||
| return None |
There was a problem hiding this comment.
Very nitpicky, but noticed this in more functions in this file. else clause and return None is reduntant, though I would argue explicitly returning None is a ncie thing to do.
| if 'daylord' in path: | |
| return 'House of Lords' | |
| elif 'debates' in path: | |
| return 'House of Commons' | |
| else: | |
| return None | |
| if 'daylord' in path: | |
| return 'House of Lords' | |
| elif 'debates' in path: | |
| return 'House of Commons' | |
| return None |
| speech_id = abbreviate_speech_id(full_speech_id) | ||
| previous_topic = '' | ||
| for key in topic_dict: | ||
| if float(key) > float(speech_id): |
There was a problem hiding this comment.
Smart way to compare 10.6 to 10.7 :)
| def lookup_person_attribute(lookup_tuple): | ||
| metadata_dict, id, name, label = lookup_tuple #name is only included for debugging purposes | ||
|
|
||
| id = id.split('/')[-1] if id else None # twfy ID is at the end of uri |
| <p-multiSelect | ||
|
|
||
| [id]="filter.corpusField.name" | ||
| [disabled]="!options.length" | ||
| [filter]="options.length>=6" | ||
| [options]="options" | ||
| [virtualScroll]="true" | ||
| [virtualScrollItemSize]="60" | ||
| [maxSelectedLabels]=1 | ||
| placeholder="Choose" | ||
| [ngModel]="data" | ||
| (onChange)="update($event.value)" ariaLabelledBy="legend-filter-{{filter.displayName | slugify}}" fluid> |
There was a problem hiding this comment.
This seems to work well (tested on acc), but is a separate feature.
The filters are well tested. Should the virtualscroll be tested there? (not sure, may be fine like this).
2 participants
Nieuwe UK Data