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.

Sign up for GitHub

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

Hugging Face Hub Upgrades #4231

Merged
merged 10 commits into from
Apr 5, 2024
Merged

Hugging Face Hub Upgrades #4231

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6cab1d0
makes fiftyone version req optional
jacobmarks Apr 4, 2024
e50e353
document min_fiftyone_version
jacobmarks Apr 4, 2024
9e1e7d5
validation min_fiftyone_version
jacobmarks Apr 4, 2024
df1db86
remove unused import
jacobmarks Apr 4, 2024
e4e78ac
adds preview_path option to preview dataset
jacobmarks Apr 4, 2024
40f7b92
documents preview_path
jacobmarks Apr 4, 2024
d5ea06e
improve name resolution
jacobmarks Apr 4, 2024
b71e3fb
only download necessary media files for FiftyOneDatasets
jacobmarks Apr 4, 2024
e2f55af
removing version validation
jacobmarks Apr 4, 2024
6937faa
improve efficiency
jacobmarks Apr 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
validation min_fiftyone_version
  • Loading branch information
@jacobmarks
jacobmarks committed Apr 4, 2024
commit 9e1e7d5b02846d65d76ffc4b939cb707be23ee67
10 changes: 10 additions & 0 deletions fiftyone/utils/huggingface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import os
from packaging.requirements import Requirement
from PIL import Image
import re
import requests

import yaml
Expand Down Expand Up @@ -305,6 +306,11 @@ def _get_fiftyone_version(self, kwargs):
"""


def _validate_fiftyone_version(version_string):
## match to "a.b.c" where a, b, c are integers
return bool(re.match(r"\d+\.\d+\.\d+", version_string))


def _populate_config_file(
config_filepath,
dataset,
Expand All @@ -321,6 +327,10 @@ def _populate_config_file(
}

if min_fiftyone_version is not None:
if not _validate_fiftyone_version(min_fiftyone_version):
Copy link
Contributor

Choose a reason for hiding this comment

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

I would recommend removing this validation as version numbers could actually contain letters and stuff (not usual, but could, eg release candidates).

How about just providing an example in the docstring?

        min_fiftyone_version (None): the minimum version of FiftyOne required,
            if applicable. For example ``"0.23"``

raise ValueError(
f"Invalid fiftyone version requirement: {min_fiftyone_version}"
)
version_val = f">={min_fiftyone_version}"
config_dict["fiftyone"] = {"version": version_val}

Expand Down