chore: Remove attrdict from model hub #8554
Conversation
✅ Deploy Preview for determined-ui canceled.
|
| self.hparams = context.get_hparams() | ||
| self.data_config = context.get_data_config() |
There was a problem hiding this comment.
I'm pretty sure this is a breaking change to MMDetTrial, since people are expected to subclass this object.
Why not just use the simplenamespace here too? Wouldn't that be enough?
There was a problem hiding this comment.
In the PyTorchTrialContext, hparams is an Optional[Dict], which is what we're passing here. Are users of MMDetTrial doing an __init__, then changing hparams after the fact? My assumption was that hparams are expected to be passed as a dict or read from a file as a dict and that's the end of it.
There was a problem hiding this comment.
This also applies to data config, i.e. def get_data_config(self) -> Dict[str, Any]
There was a problem hiding this comment.
I can change this PR to change all dict objects to SimpleNamespace, but I'm not sure what the resulting benefit is.
MikhailKardash
force-pushed
the
model_hub_attrdict_2
branch
from
cfa3b6b
to
baf4c6e
Compare
Description
attrdictis broken since Python 3.9. Possible replacements do not meet our dependency requirements. This PR implements our ownAttrDictas a utility class.Replaces #8541
Test Plan
Unit tests pass. Also try any
mmdetectionandhugginfaceexample.Checklist
docs/release-notes/.See Release Note for details.
Ticket
MLG-1442