[tune] Remove rllib dep again, and add a test #1792
Conversation
|
what was the current issue? don't the failed imports get caught in |
|
Oh, it looks like the try catch is gone there. Hm, we could put it back, but this is probably a cleaner fix. |
|
Test FAILed. |
|
Test PASSed. |
|
Test PASSed. |
python/ray/tune/trial.py
Outdated
| raise TuneError("Unknown trainable: " + trainable_name) | ||
| if not has_trainable(trainable_name): | ||
| # Make sure rllib agents are registered | ||
| from ray import rllib # noqa # pylint: disable=unused-import |
There was a problem hiding this comment.
Nit, the comment should just be # noqa: F401
| @@ -0,0 +1,26 @@ | |||
| #!/usr/bin/env python | |||
There was a problem hiding this comment.
This is fine if this is the only thing that's ever going to be in this test file. However in general new tests should all use pytest.
There was a problem hiding this comment.
Yeah this one kinda has to be its own file since it tests import state.
|
Test PASSed. |
What do these changes do?
Load rllib as late as possible and only if needed (for RLlib agents). Add a test to make sure this doesn't regress again.
cc @vlad17