Skip to content
This repository was archived by the owner on Mar 16, 2022. It is now read-only.

Enum and dataclasses #2

Closed
mthrok opened this issue Sep 28, 2021 · 2 comments
Closed

Enum and dataclasses #2

mthrok opened this issue Sep 28, 2021 · 2 comments
Labels
enhancement New feature or request

Comments

@mthrok
Copy link
Collaborator

mthrok commented Sep 28, 2021

(Opening a new issue so as not to mix the ongoing discussion)

I feel that separating dataclass and Enum make the code more readable.

@dataclass
class ResNet50Weights(Enum):
    ImageNet1K_RefV1 = (
        url="https://path/to/weights.pth",  # Weights URL/path
        transforms=partial(ImageNetPreprocessing, width=224),  # Preprocessing transform constructor
        meta={"num_classes": 1000, "Acc@1": 76.130, "classes": [...]}  # Arbitrary Meta-Data
        # Other customizable fields go here
    )
@dataclass
class ResNet50Config:
    url: str
    transforms: Any
    meta: meta: Dict[str, Any]
    latest: bool


class ResNet50Weights(Enum):
    ImageNet1K_RefV1 = ResNet50Config(...)
    ...
@datumbox
Copy link
Owner

Thanks @mthrok.

Your question/proposal is valid just note that the code you are quoting comes from the README and its pseudocode, not real code. It was there for illustration purposes to keep things brief.

Here is the actual implementation:

@dataclass
class Weights(Enum):
    url: str
    transforms: Callable
    meta: Dict[str, Any]
    latest: bool

    # method definitions go here

And the here is the actual usage:

class ResNet50Weights(Weights):
ImageNet1K_RefV1 = (
'https://download.pytorch.org/models/resnet50-0676ba61.pth',
partial(ImageNetEval, crop_size=224),
{'size': (224, 224), 'classes': ImageNet.classes,
'recipe': 'https://github.com/pytorch/vision/tree/main/references/classification#resnext-50-32x4d'},
False
)

So as you see with this approach, the parameters are not named.

The alternative implementation would look like the following. Note that the use of the base Weights class is now optional as the methods can live in the flat namespace instead but for reasons I mention at #1 (comment) I think it's still valuable to have it (I'm open to feedback, happy to change it if lots of people think it needs to go):

class Weights(Enum):
    # method definitions go here

@dataclass
class WeightEntry:
    url: str
    transforms: Any
    meta: meta: Dict[str, Any]
    latest: bool

Usage:

class ResNet50Weights(Weights):
    ImageNet1K_RefV1 = WeightEntry(url=..., transforms=..., meta=..., latest=True)

@datumbox
Copy link
Owner

datumbox commented Oct 1, 2021

@mthrok We are potentially having a duplicate discussion for this on #7. Shall we close this and continue the chat there?

@datumbox datumbox added discussion-needed Open for discussion waiting-response Response required by op and removed discussion-needed Open for discussion labels Oct 1, 2021
@datumbox datumbox added enhancement New feature or request and removed waiting-response Response required by op labels Oct 3, 2021
@datumbox datumbox closed this as completed Oct 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants