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

Check that keys of nested dictionaries in annotations are str #1399

Merged
merged 2 commits into from
Feb 22, 2024

Conversation

zm711
Copy link
Contributor

@zm711 zm711 commented Feb 11, 2024

Fixes #1195.

Just adding a check for the key. Example from issue:

my_meta={(lambda a: None): 5}

Because the keys aren't checked this doesn't fail until writing. We should check to make sure the key is one of the allowable annotations.

We still have problems with writing some of the allowable annotations (see this issue), but adding this check at least moves in the right direction.

@zm711 zm711 changed the title check key in annotations Check that keys of nested dictionaries in annotations are of allowable annotation types Feb 11, 2024
@zm711 zm711 added the Core label Feb 16, 2024
@apdavison
Copy link
Member

I think we'd always assumed that annotation keys would be strings, since they represent names. I don't think anyone would be surprised if they got an error when using a lambda function as the name for an annotation.

If we're going to check this, then only str should be allowed for annotation keys and sub-keys, with a TypeError raised otherwise.

@zm711
Copy link
Contributor Author

zm711 commented Feb 19, 2024

That makes things easier, so I'm happy to switch that :)

@zm711 zm711 changed the title Check that keys of nested dictionaries in annotations are of allowable annotation types Check that keys of nested dictionaries in annotations are str Feb 19, 2024
@apdavison apdavison merged commit 620a2a8 into NeuralEnsemble:master Feb 22, 2024
41 checks passed
@zm711 zm711 deleted the check-key branch February 22, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

check_annotations doesn't check dict key type
2 participants