Skip to content

Conversation

@fyang3
Copy link
Contributor

@fyang3 fyang3 commented May 18, 2021

  • added Honorifics
  • char_list and char_disamb
  • temporarily resolve the proximity/init dependency issue

- added Honorifics
- char_list and char_disamb
- temporarily resolve the proximity/init dependency issue
@fyang3 fyang3 requested a review from MBJean May 18, 2021 04:30
Copy link
Contributor

@MBJean MBJean left a comment

Choose a reason for hiding this comment

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

Overall, great work! I've left some comments below, including a few things that I think should be changed. Let me know what you think.

from gender_analysis.analysis.dependency_parsing import *
from gender_analysis.analysis.dunning import *
from gender_analysis.analysis.gender_frequency import *
from gender_analysis.analysis.instance_distance import *
Copy link
Contributor

Choose a reason for hiding this comment

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

A note for posterity. This is a temporary measure to prevent circular imports caused by proximity.py importing Corpus for type hinting. PR #163 attempts to address the issue more fundamentally.

set(self.filter_honr(char_list[j][0]))):
char_cluster.append(char_list[j])
to_return.append(char_cluster)
return to_return
Copy link
Contributor

Choose a reason for hiding this comment

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

As we talked about in Slack briefly, this method probably requires some thinking through. If I'm reading the test output in 710 correctly, it looks like the disambiguation is overly generous, and I suspect we can figure out a more optimized way to traverse those character lists. Let's chat through some issues in office hours.

fyang3 added 3 commits May 21, 2021 15:34
added rough draft for coref_resolution
draft for simple HCI-console-based approach for disambiguate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants