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

Deprecated ObjectConf for TargetConf #882

Closed
omry opened this issue Aug 16, 2020 · 2 comments · Fixed by #883
Closed

Deprecated ObjectConf for TargetConf #882

omry opened this issue Aug 16, 2020 · 2 comments · Fixed by #883
Milestone

Comments

@omry
Copy link
Collaborator

omry commented Aug 16, 2020

This is a partial implementation of #566.

TLDR

Object configs is changing from:

target: classname
params:
  a: 10
  b: 20

to:

_target_: classname
a: 10
b: 20

This include Hydra's launchers and sweepers, so if they appear in your config file you will need to adjust the configs.

Details

ObjectConf:

@dataclass
class ObjectConf(Dict[str, Any]):
    # class, class method or function name
    target: str = MISSING

    # parameters to pass to cls when calling it
    params: Any = field(default_factory=dict)

TargetConf:

@dataclass
class TargetConf:
    """
    Use by subclassing and adding fields
    e.g:
    @dataclass
    class UserConf(TargetConf):
        _target_ = "module.User"
        name: str = MISSING
        age: int = MISSING
    """

    # class, class method or function name
    _target_: str = MISSING

This will be deprecated for user code, but for Hydra it's a breaking change because sweeper and launchers will no longer need the pesky params to override their parameters:

$ python foo.py hydra.sweeper.params.max_batch_size=2
# will become
$ python foo.py hydra.sweeper.max_batch_size=2
@omry omry added this to the 1.0.0 milestone Aug 16, 2020
@omry omry changed the title Deprecated ObjectConf for Target Conf Deprecated ObjectConf for TargetConf Aug 16, 2020
@omry
Copy link
Collaborator Author

omry commented Aug 16, 2020

Possible alternative designs/names for TargetConf.
Happy to discuss this while it's still hot.

# 1
@dataclass
class TargetConf:
    # class, class method or function name
    _target_: str = MISSING

#2 
@dataclass
class CallConf:
    _callable_: str = MISSING

#3 
@dataclass
class CallConf:
   # one of:
   _class_ : str = MISSING      # class only
   _function_ : str = MISSING   # function only
   _callable_: str = MISSING    # either class or function

@shagunsodhani
Copy link
Contributor

My vote is for 1. It is clean, simpler and unambiguous.

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 a pull request may close this issue.

2 participants