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

Compare mc #20

Merged
merged 7 commits into from
Oct 4, 2021
Merged

Compare mc #20

merged 7 commits into from
Oct 4, 2021

Conversation

jasonho-lynx
Copy link
Contributor

  • Added compare.html.jinja template
  • Added ModelCardToolkit.compare_model_cards() and its prerequisite methods.
  • Added a ModelCardToolkit._get_jinja_template() method, modified existing rendering steps to use this
  • Cleaned up ModelCard.get_test_results(), which is used in Github Actions (recall # TODO: clean up this mess lol knew I missed out some cleanups previously but just couldn't remember)

Sample Usage

Open up an example notebook, run all cells, then at the end, run a new cell with this:

from copy import deepcopy

# mc is the existing model card created from the example notebook
mc2 = deepcopy(mc)
mc2.model_details.name = 'another credit card model'

comparison = mct.compare_model_cards(mc, mc2)
display.display(display.HTML(comparison))

A plain h1 tag will be rendered if no similarities are found:

nothing = mct.compare_model_cards(mc, ModelCard())
display.display(display.HTML(nothing))

# returns '<h1 style="text-align: left;"> No common reports found </h1>'

@@ -133,6 +137,30 @@ def _read_proto_file(self, path: Text) -> ModelCard:
model_card_proto.ParseFromString(f.read())
return ModelCard().copy_from_proto(model_card_proto)

def _get_jinja_template(
self, template_path: str = None, template_file_name: str = None
Copy link
Contributor

Choose a reason for hiding this comment

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

how about assigning to template_path: str = _DEFAULT_UI_TEMPLATE_FILE? Then no need for template_file_name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

template_path takes more than just the file name constant though, if you see line 226-229 of the old version of this file:

        if not template_path:
            template_path = os.path.join(
                self._mcta_template_dir, _DEFAULT_UI_TEMPLATE_FILE
            )

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case, it was a dir and file. In this case, if you are just using path then there's no need for file_name

@jasonho-lynx jasonho-lynx merged commit 4dbca0f into main Oct 4, 2021
@jasonho-lynx jasonho-lynx deleted the compare-mc branch October 4, 2021 11:01
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.

2 participants