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

Model info sanity #368

Merged
merged 3 commits into from Nov 4, 2017
Merged

Model info sanity #368

merged 3 commits into from Nov 4, 2017

Conversation

halfak
Copy link
Member

@halfak halfak commented Nov 3, 2017

Some improvements to the way that ModelInfo is handled.

  • Fixes key pattern in format_str for ModelInfo
  • Impossible threshold optimizations no longer blow up.
  • Adds ModelInfoLookupError and tests.

for key in path_tree or self.normalize_fields(path_tree):
if hasattr(self[key], "format_str"):
for key in self.normalize_fields(path_tree):
key_val = try_key(key, self)
Copy link
Member Author

Choose a reason for hiding this comment

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

Note to reviewer, "try_key()" is where the magic happens.

for key in path_tree or self.normalize_fields(path_tree):
if hasattr(self[key], "format_str"):
for key in self.normalize_fields(path_tree):
key_val = try_key(key, self)
Copy link
Contributor

Choose a reason for hiding this comment

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

We no longer test that key_val has .format_str

return d[int(key)]
except ValueError:
raise e
except KeyError:
Copy link
Contributor

Choose a reason for hiding this comment

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

contains could call try_key internally, to ensure consistent logic.

@@ -35,6 +35,7 @@ def test_sts():
print(skewed_sts.format_str({}, threshold_ndigits=3))
print(balanced_sts.format_str({}, threshold_ndigits=3))
print(skewed_sts.format_str({'maximum recall @ precision >= 0.9': {}}))
print(skewed_sts.format_str({'maximum recall @ precision >= 1.1': {}}))
Copy link
Contributor

Choose a reason for hiding this comment

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

O_o

@adamwight adamwight merged commit c30da92 into master Nov 4, 2017
@adamwight adamwight deleted the model_info_sanity branch November 4, 2017 14:43
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.

None yet

2 participants