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

Validate for nouny features on verby morphology #623

Closed
emilymbender opened this issue Jan 31, 2022 · 5 comments
Closed

Validate for nouny features on verby morphology #623

emilymbender opened this issue Jan 31, 2022 · 5 comments

Comments

@emilymbender
Copy link
Contributor

We should give validation warnings (at least) when the user tries to say that "person = 3rd" on a verb or auxiliary.

@ltxom
Copy link
Contributor

ltxom commented May 11, 2022

I see there are code snippets in the lexicon.py that implemented this validation:

index_features = set(['person', 'number', 'gender'])
if lextype in ('verb', 'adj', 'cop'):
        ...
	elif feat.get('head') in ('verb', 'adj', 'cop'):
		if feat.get('name') in index_features:
			mess = 'This feature is associated with nouns. ' + \
				   'Please select one of the NP options to use this feature.'
			vr.err(feat.full_key + '_head', mess)

If PNG features are specified on verb, adj, or cop, it shows an error message. This logic can also apply to aux, but a comment says, "lap: 1/5/11 removed aux to get rid of overzealous validation." However, I think PNG features cannot apply to auxiliary, so we should block it as well?

Also, should we add 'case' into index_features other than PNG?

@emilymbender
Copy link
Contributor Author

Yes, I suspect that whatever was "overzealous" back then isn't anymore. (Though maybe we'll find out!)

Where else is the variable index_features used? It's not correct to call CASE an index_features (it doesn't appear on ref-ind), so it might be better to invoke CASE separately (as in check the union of index_features and {CASE}).

@ltxom
Copy link
Contributor

ltxom commented May 11, 2022

I got it! I will add aux back to the checklist.

The variable index_features is only used to check if the feature in this set is associated with nouns. But for clarity purposes, I will check CASE separately.

@emilymbender
Copy link
Contributor Author

Sounds good!

ltxom added a commit to ltxom/matrix that referenced this issue May 11, 2022
@ltxom ltxom mentioned this issue May 11, 2022
emilymbender added a commit that referenced this issue Jun 8, 2022
@ltxom
Copy link
Contributor

ltxom commented Jun 9, 2022

This issue can be closed :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants