Allow instantiation from class methods #498

dhpollack · 2020-03-18T15:45:25Z

instantiate an object from a class method of that object
add tests for this
simplify get_static_method function

Signed-off-by: David Pollack d.pollack@solvemate.com

Motivation

I wanted to use hydra with the huggingface transfromers library and they extensively use a class method named from_pretrained.

Currently one could do this by using the get_static_method and then manually converting the params key and passing them to the gotten method. However that's not very straight forward or intuitive and this doesn't add much logic to the utility function.

Have you read the Contributing Guidelines on pull requests?

Yes - passed tests in test_utils.py and flake8

Test Plan

I added a test that tests the basic case.

omry

Hi, thanks for the PR.

See my comments.

hydra/utils.py

tests/test_utils.py

omry

Starting to look good.

hydra/utils.py

tests/test_utils.py

website/docs/patterns/objects.md

omry · 2020-03-21T17:32:29Z

As for the failing CI:
see the developer guide for info on how to run the tests and the lint locally.

lgtm-com · 2020-03-23T20:43:06Z

This pull request introduces 1 alert when merging 52e99cc into 67a525b - view on LGTM.com

new alerts:

1 for Except block handles 'BaseException'

hydra/utils.py

website/docs/patterns/objects.md

omry · 2020-03-26T04:33:04Z

ping

dhpollack · 2020-03-26T18:33:11Z

The test failues look like they are due to the upgrade in omegaconf. Do you prefer a git rebase or a git merge on the master? I tested both on branches locally. I would assume rebasing, but I figured I'd ask.

omry · 2020-03-26T19:40:39Z

I prefer rebase.

* instantiate an object from a class method of that object * add tests for this * simplify get_static_method function Signed-off-by: David Pollack <d.pollack@solvemate.com>

* works with class, classmethod, or staticmethod * still requires the user to specify the module * simplifies importing of all types into a single method Signed-off-by: David Pollack <david@da3.net>

Signed-off-by: David Pollack <david@da3.net>

this is a reimplementation of the pydoc.locate function. That function would attempt to load a module from beginning to end and if any link in the chain did not load properly then it would fail. The reimplemented function searches for the module to load from the end of the path to the beginning. Additionally, this reimplementation will raise exceptions instead of returning None if the path cannot be located. Signed-off-by: David Pollack <david@da3.net>

omry

Thanks. A few more comments about the docs and the tests.

hydra/utils.py

website/docs/patterns/objects.md

tests/test_utils.py

omry

awesome. almost there.

hydra/utils.py

website/docs/patterns/objects.md

dhpollack · 2020-03-28T22:26:49Z

Alright, I think that I solved the mypy issues by using _instantiate_class for classes and _call_callable for functions and methods. I also created a bunch of aliases for the old naming scheme and renamed functions as well. I also added a few extra tests for builtins and nested methods.

As for the documentation, let me know what you think. I tried to keep it as compact as possible, but may be doing too many things with the same class.

P.S. the win37 failure looks unrelated to this PR. Looks like the conda servers glitched.

omry

I would like to split hydra.utils into hydra.utils for public API and some other internal class with non public API. but this can be a followup diff.

Made one last suggestion.

website/docs/patterns/objects.md

Co-Authored-By: Omry Yadan <omry@fb.com>

omry · 2020-03-29T01:00:38Z

As for the docs, they look good here but I would like to see them in the website.
Did you start the website locally to test that they look okay?

dhpollack · 2020-03-29T01:02:29Z

I didn't but I'll check it tomorrow

Split the current objects docs into two sections. Moved the call section to the top and the db example to the bottom. Renamed the header to reflect the ability to call functions and methods.

omry · 2020-03-29T16:26:44Z

Awesome. edited the doc a bit and added the issue #505 for API cleanup before 1.0.

omry · 2020-03-30T07:59:35Z

Ohh, forgot.
Can you add a news fragment file?
see the developer guide for details.

omry · 2020-03-31T00:07:35Z

Awesome, thanks David!

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 18, 2020

omry requested changes Mar 18, 2020

View reviewed changes

hydra/utils.py Outdated Show resolved Hide resolved

tests/test_utils.py Outdated Show resolved Hide resolved

dhpollack force-pushed the dhp/instantiate_from_class_method branch from bd36485 to 1439acc Compare March 21, 2020 13:53

omry requested changes Mar 21, 2020

View reviewed changes

hydra/utils.py Outdated Show resolved Hide resolved

hydra/utils.py Outdated Show resolved Hide resolved

tests/test_utils.py Show resolved Hide resolved

website/docs/patterns/objects.md Outdated Show resolved Hide resolved

omry reviewed Mar 23, 2020

View reviewed changes

hydra/utils.py Outdated Show resolved Hide resolved

hydra/utils.py Outdated Show resolved Hide resolved

hydra/utils.py Outdated Show resolved Hide resolved

hydra/utils.py Outdated Show resolved Hide resolved

website/docs/patterns/objects.md Outdated Show resolved Hide resolved

omry force-pushed the master branch from 67a525b to aa00a57 Compare March 24, 2020 23:52

David Pollack and others added 6 commits March 26, 2020 21:01

Allow instantiation from class methods

eecd24a

* instantiate an object from a class method of that object * add tests for this * simplify get_static_method function Signed-off-by: David Pollack <d.pollack@solvemate.com>

pydoc.locate to instantiate

cbdc577

* works with class, classmethod, or staticmethod * still requires the user to specify the module * simplifies importing of all types into a single method Signed-off-by: David Pollack <david@da3.net>

Use f-strings

71b460b

Signed-off-by: David Pollack <david@da3.net>

fix documentation example when instantiating from classmethod

e9bc1a8

linting and formatting

600f5a8

dhpollack force-pushed the dhp/instantiate_from_class_method branch from cbe1a65 to ae26554 Compare March 26, 2020 20:01

omry requested changes Mar 27, 2020

View reviewed changes

hydra/utils.py Outdated Show resolved Hide resolved

website/docs/patterns/objects.md Outdated Show resolved Hide resolved

tests/test_utils.py Outdated Show resolved Hide resolved

update docs and make changes based on PR comments

6d57655

dhpollack force-pushed the dhp/instantiate_from_class_method branch from ae26554 to 6d57655 Compare March 28, 2020 22:07

fix typo in variable name

cf80cb1

omry reviewed Mar 28, 2020

View reviewed changes

hydra/utils.py Outdated Show resolved Hide resolved

website/docs/patterns/objects.md Outdated Show resolved Hide resolved

fixes based on comments

d9d599c

omry reviewed Mar 28, 2020

View reviewed changes

website/docs/patterns/objects.md Outdated Show resolved Hide resolved

Update website/docs/patterns/objects.md

7dce374

Co-Authored-By: Omry Yadan <omry@fb.com>

change order of objects docs

66533c9

Split the current objects docs into two sections. Moved the call section to the top and the db example to the bottom. Renamed the header to reflect the ability to call functions and methods.

Update objects.md

b4f85aa

omry approved these changes Mar 29, 2020

View reviewed changes

news item for hydra.utils.call()

cf29c5f

omry merged commit a5bd89a into facebookresearch:master Mar 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow instantiation from class methods #498

Allow instantiation from class methods #498

dhpollack commented Mar 18, 2020

omry left a comment •

edited

Loading

omry left a comment

omry commented Mar 21, 2020

lgtm-com bot commented Mar 23, 2020

omry commented Mar 26, 2020

dhpollack commented Mar 26, 2020

omry commented Mar 26, 2020

omry left a comment

omry left a comment

dhpollack commented Mar 28, 2020

omry left a comment

omry commented Mar 29, 2020

dhpollack commented Mar 29, 2020

omry commented Mar 29, 2020

omry commented Mar 30, 2020

omry commented Mar 31, 2020

Allow instantiation from class methods #498

Allow instantiation from class methods #498

Conversation

dhpollack commented Mar 18, 2020

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

omry left a comment • edited Loading

Choose a reason for hiding this comment

omry left a comment

Choose a reason for hiding this comment

omry commented Mar 21, 2020

lgtm-com bot commented Mar 23, 2020

omry commented Mar 26, 2020

dhpollack commented Mar 26, 2020

omry commented Mar 26, 2020

omry left a comment

Choose a reason for hiding this comment

omry left a comment

Choose a reason for hiding this comment

dhpollack commented Mar 28, 2020

omry left a comment

Choose a reason for hiding this comment

omry commented Mar 29, 2020

dhpollack commented Mar 29, 2020

omry commented Mar 29, 2020

omry commented Mar 30, 2020

omry commented Mar 31, 2020

omry left a comment •

edited

Loading