added autoeval fields to repocard types #901

TristanThrush · 2022-06-10T20:26:00Z

This PR makes the repocard types consistent with the spec that autoeval uses: https://raw.githubusercontent.com/huggingface/hub-docs/main/modelcard.md

It is based on lewis's PR here, but it only includes the necessary changes to repocard_types.py

HuggingFaceDocBuilderDev · 2022-06-10T20:29:07Z

The documentation is not available anymore as the PR was closed or merged.

lewtun

Thanks a lot for adding this - LGTM!

Let's wait for a review from the huggingface_hub maintainers before merging this.

cc @LysandreJik @julien-c

src/huggingface_hub/repocard_types.py

lewtun · 2022-06-13T19:04:57Z

src/huggingface_hub/repocard_types.py

    task: "SingleResultTask"
    dataset: "Optional[SingleResultDataset]"
+    metrics: "List[SingleMetric]"


For reviewers: this ordering of task > dataset > metrics is more natural and easier to review during Hub PRs

julien-c

looks good to me!

adrinjalali · 2022-06-14T08:27:51Z

cc @nateraw

nateraw

Might be nice to show how to use this in one of the docstrings in repocard.py, but other than that looks good to me.

nateraw · 2022-06-14T17:45:37Z

tests/test_repocard.py

+    - type: accuracy
+      value: 0.2662102282047272
+      name: Accuracy
+      verified: false


nit - should we add config in example here too, since you added that with this PR?

Thanks for this feedback!

In order to do this, were you thinking that I should also update the inputs to metadata_eval_result function here: https://github.com/huggingface/huggingface_hub/blob/main/src/huggingface_hub/repocard.py#L80

An additional change like this is needed, because the test here won't pass otherwise:
https://github.com/huggingface/huggingface_hub/blob/main/tests/test_repocard.py#L166

I think that's what you were thinking. I just pushed a change where I added optional fields to metadata_eval_result and added documentation. lmk what you think!

lewtun · 2022-06-16T13:52:36Z

Hi @adrinjalali can you please advise us on how to handle the failing codecov test? Is it a question of writing more unit tests for the repocard or something else?

lewtun · 2022-06-16T19:29:03Z

In order to migrate the AutoTrain backend to use #888 instead of #884, we'll actually need the changes in this PR to be available in @SBrandeis' fork/branch. (We rely on the additional metadata fields provided by the current PR)

To unblock @coyotte508 in https://github.com/huggingface/moon-landing/pull/3144 (which disables the upload_file() endpoint we use in AutoTrain) one possibility is to:

Close added autoeval fields to repocard types #901 and open an identical one on @SBrandeis's branch that takes into account the reviewer feedback.
Migrate the AutoTrain backend to use ✨ New create_commit API #888 once step (1) is merged

This isn't a pretty solution and it pollutes #888 with changes unrelated to the new API.

An alternative would be to:

Allow https://github.com/huggingface/moon-landing/pull/3144 to disable the upload_file() endpoint we use in AutoTrain. This will disable the evaluation service until ✨ New create_commit API #888 is merged
Merge added autoeval fields to repocard types #901
Migrate the AutoTrain backend to the new commit API once ✨ New create_commit API #888 is merged

In the interest of releasing a stable evaluation service without lots of hacks, I'm in favour of the second option as long as it doesn't delay the release by too much. @SBrandeis do you have a rough idea how long it will take to get your PR merged?

cc @douwekiela @TristanThrush @abhishekkrthakur

lewtun · 2022-06-17T10:00:11Z

After discussing internally, we'll go for the option to wait until #888 is merged before migrating the AutoTrain backend.

@coyotte508 from my side: feel free to merge https://github.com/huggingface/moon-landing/pull/3144 when you want :)

coyotte508 · 2022-06-17T19:24:58Z

@lewtun https://github.com/huggingface/moon-landing/pull/3144 is ready to be merged

Nothing's going to burn on your side, right? 😅

lewtun · 2022-06-17T19:41:38Z

Nothing's going to burn on your side, right? 😅

Nope, the eval service is still in dev but let's quickly ask @gante for confirmation since I believe he was using my branch in transformers

gante · 2022-06-17T19:52:08Z

Don't mind transformers, I'm the sole user of the related functionality and I can hold it for a few days.

I will make the appropriate changes as soon as #888 gets merged 👍

* Add errors * Style * Review * hf_hub_download support * Errors for hf_hub_download * Typo * Handle 401 error * Tests for 401 error * Typo * Review comments

…data_eval_result, added documentation

@XciD

* 🏗 Use `hub-ci` for tests cc @XciD * 🩹 Also update URL for staging mode * ✅ 401 is raised when the user is not authenticated * 🗑 Deprecare `identical_ok` * Longer deprecation period * ✅ Fix the last failing test * Warning match docstring

* fix for spaces * fix for spaces * removed creating repository and added warning * revert my changes * added tests * removed debugger 😐 * fixed repository removal * Added tests and error * import pytest * fixed tests * fixed tests * style * removed repo removal * make style * fixed test with Lysandres patch * added fix

* Remove deprecations * Typo * Update src/huggingface_hub/README.md Co-authored-by: Julien Chaumond <julien@huggingface.co> Co-authored-by: Julien Chaumond <julien@huggingface.co>

* Add request ID to all requests * Typo * Typo * Review comments * Update src/huggingface_hub/utils/_errors.py Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com> Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>

* Invert deprecation for create_repo * Set to 0.10

Co-authored-by: Lyle Nel <lyle@owlin.com>

Co-authored-by: lewtun <lewis.c.tunstall@gmail.com>

…x/add_autoeval_fields_to_repocard_types

adrinjalali

Hi @adrinjalali can you please advise us on how to handle the failing codecov test? Is it a question of writing more unit tests for the repocard or something else?

codecov runs two tests, one to check the percentage of tests in the PR's patch, the other one on the whole project. For this PR, the only thing left to test now are metrics_config and metrics_verified.

adrinjalali · 2022-06-19T09:48:26Z

src/huggingface_hub/repocard.py

+    """
+    Creates a metadata dict with the result from a model evaluated on a dataset.
+
+    Example:


the example section should come after the Returns section

Thanks, added setting of metrics_config and metrics_verified to tests and did the reordering suggestion that you mentioned for the documentation.

…verified to tests

…x/add_autoeval_fields_to_repocard_types

* fix link * `verified` should be omitted when false * move docstring to its actual attribute * Attempt to show that `args` should be a dict

added autoeval fields to repocard types

c7c88f7

TristanThrush marked this pull request as draft June 10, 2022 23:47

modified test, linted

6947d35

TristanThrush marked this pull request as ready for review June 13, 2022 17:49

TristanThrush requested a review from lewtun June 13, 2022 17:49

lewtun approved these changes Jun 13, 2022

View reviewed changes

julien-c reviewed Jun 14, 2022

View reviewed changes

nateraw approved these changes Jun 14, 2022

View reviewed changes

TristanThrush requested a review from nateraw June 17, 2022 23:06

lsb and others added 12 commits June 19, 2022 12:07

Fix typo (#902)

8b34788

Refine 404 errors (#878)

ddb266c

* Add errors * Style * Review * hf_hub_download support * Errors for hf_hub_download * Typo * Handle 401 error * Tests for 401 error * Typo * Review comments

added documentation for the repocard change in repocard, updated meta…

917a00f

…data_eval_result, added documentation

🏗 Use hub-ci for tests (#898)

18e000f

* 🏗 Use `hub-ci` for tests cc @XciD * 🩹 Also update URL for staging mode * ✅ 401 is raised when the user is not authenticated * 🗑 Deprecare `identical_ok` * Longer deprecation period * ✅ Fix the last failing test * Warning match docstring

Remove deprecations (#910)

043af01

* Remove deprecations * Typo * Update src/huggingface_hub/README.md Co-authored-by: Julien Chaumond <julien@huggingface.co> Co-authored-by: Julien Chaumond <julien@huggingface.co>

Invert deprecation for create_repo (#912)

bbd8553

* Invert deprecation for create_repo * Set to 0.10

Constant was accidentally removed during deprecation transition (#913)

c86c39a

Co-authored-by: Lyle Nel <lyle@owlin.com>

Update src/huggingface_hub/repocard_types.py

b5fe78d

Co-authored-by: lewtun <lewis.c.tunstall@gmail.com>

Update repocard_types.py

3ab9555

Update repocard_types.py

74ace9d

TristanThrush force-pushed the fix/add_autoeval_fields_to_repocard_types branch from 6674175 to 74ace9d Compare June 19, 2022 19:09

Merge branch 'main' of github.com:huggingface/huggingface_hub into fi…

8adda6f

…x/add_autoeval_fields_to_repocard_types

adrinjalali reviewed Jun 20, 2022

View reviewed changes

TristanThrush added 2 commits June 20, 2022 23:18

reorded metadata_eval_request_docs, added metrics_config and metrics_…

e769f73

…verified to tests

Merge branch 'main' of github.com:huggingface/huggingface_hub into fi…

46bb31b

…x/add_autoeval_fields_to_repocard_types

adrinjalali merged commit 80d4a9e into main Jun 21, 2022

adrinjalali deleted the fix/add_autoeval_fields_to_repocard_types branch June 21, 2022 07:27

julien-c mentioned this pull request Jun 27, 2022

Followup to #901: Tweak repocard_types.py #931

Merged

julien-c added a commit that referenced this pull request Jun 27, 2022

Followup to #901: Tweak repocard_types.py (#931)

f47eabe

* fix link * `verified` should be omitted when false * move docstring to its actual attribute * Attempt to show that `args` should be a dict

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added autoeval fields to repocard types #901

added autoeval fields to repocard types #901

TristanThrush commented Jun 10, 2022 •

edited

Loading

HuggingFaceDocBuilderDev commented Jun 10, 2022 •

edited

Loading

lewtun left a comment

lewtun Jun 13, 2022

julien-c left a comment

adrinjalali commented Jun 14, 2022

nateraw left a comment

nateraw Jun 14, 2022

TristanThrush Jun 17, 2022

TristanThrush Jun 17, 2022

lewtun commented Jun 16, 2022

lewtun commented Jun 16, 2022

lewtun commented Jun 17, 2022

coyotte508 commented Jun 17, 2022 •

edited

Loading

lewtun commented Jun 17, 2022

gante commented Jun 17, 2022

adrinjalali left a comment

adrinjalali Jun 19, 2022

TristanThrush Jun 21, 2022

added autoeval fields to repocard types #901

added autoeval fields to repocard types #901

Conversation

TristanThrush commented Jun 10, 2022 • edited Loading

HuggingFaceDocBuilderDev commented Jun 10, 2022 • edited Loading

lewtun left a comment

Choose a reason for hiding this comment

lewtun Jun 13, 2022

Choose a reason for hiding this comment

julien-c left a comment

Choose a reason for hiding this comment

adrinjalali commented Jun 14, 2022

nateraw left a comment

Choose a reason for hiding this comment

nateraw Jun 14, 2022

Choose a reason for hiding this comment

TristanThrush Jun 17, 2022

Choose a reason for hiding this comment

TristanThrush Jun 17, 2022

Choose a reason for hiding this comment

lewtun commented Jun 16, 2022

lewtun commented Jun 16, 2022

lewtun commented Jun 17, 2022

coyotte508 commented Jun 17, 2022 • edited Loading

lewtun commented Jun 17, 2022

gante commented Jun 17, 2022

adrinjalali left a comment

Choose a reason for hiding this comment

adrinjalali Jun 19, 2022

Choose a reason for hiding this comment

TristanThrush Jun 21, 2022

Choose a reason for hiding this comment

TristanThrush commented Jun 10, 2022 •

edited

Loading

HuggingFaceDocBuilderDev commented Jun 10, 2022 •

edited

Loading

coyotte508 commented Jun 17, 2022 •

edited

Loading