Create __repr__ for site_config classes

[BrianJKoopman]

Description

This PR creates __repr__ methods for each of the site_config.py classes. The returned string can be used to recreate the Config object with the exception of including the parent, which we leave out. (It created quite large representations.) (On that note -- why do we track the parent? As pointed out in the docs, it's never used.)

Questions

I had one question/comment about these classes while working on this. Why do we have all of these alternative constructor classmethods when mostly there is only one way (.from_dict()) to construct the object anyway?

I thought it would make more sense to have the constructor in .from_dict() be the default constructor, as that would clean up the __repr__ a bit. This leaves SiteConfig.from_yaml() as the only alternative constructor.

I was going to include in this PR, but it makes things sort of messy for reviewing the __repr__ work. You can see this rework here.

If that makes sense, I can PR that change as well.

Motivation and Context

A few times recently I've needed to debug something related to these objects and having a way to easily print their contents is extremely useful in that case.

How Has This Been Tested?

I wrote some tests while doing this, which use a shared dummy config that is written in the test. They mostly just assert that cfg.data == data essentially, which isn't particularly useful, but does test the objects can be made...

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.

[mhasself]

So ... yes, I think it's safe at this point to remove .from_dict and just make that the default constructor.

Can you consider this approach, instead of caching the input data dict?

    def __repr__(self):
        return f'SiteConfig.from_dict({self.__dict__})'

The problems with caching data are that the input dict and the instance attributes can get out of sync. In practice that's not very likely. But it would be confusing to have the repr be inaccurate. It would also be dangerous to have consumers of this config info start to use .whatever and .data['whatever'] interchangeably.

Some of the site_config classes will be quite amenable to the .__dict__ approach (SiteConfig), while some might require a bit of special treatment (e.g. CrossbarConfig, where the attributes don't match the input dict keys). Special treatment could include: suppressing some keys in the repr, renaming some keys in the repr, changing the attributes to better match the dict keys (this might be a bigger project though, assuming those attributes get used).

[BrianJKoopman]

So ... yes, I think it's safe at this point to remove .from_dict and just make that the default constructor.

Great, how do you want me to handle that with respect to this PR? Include it? Separate? Before/After this one? (Hopefully after, since I've done that already on top of this branch.)

Can you consider this approach, instead of caching the input data dict?

    def __repr__(self):
        return f'SiteConfig.from_dict({self.__dict__})'

The problems with caching data are that the input dict and the instance attributes can get out of sync. In practice that's not very likely. But it would be confusing to have the repr be inaccurate. It would also be dangerous to have consumers of this config info start to use .whatever and .data['whatever'] interchangeably.

Some of the site_config classes will be quite amenable to the .__dict__ approach (SiteConfig), while some might require a bit of special treatment (e.g. CrossbarConfig, where the attributes don't match the input dict keys). Special treatment could include: suppressing some keys in the repr, renaming some keys in the repr, changing the attributes to better match the dict keys (this might be a bigger project though, assuming those attributes get used).

I see your point about using the cached data attribute, but also think it's unlikely in practice that that falls out of sync.

Some thoughts on using .__dict__:

• I'm not sure changing attributes to match the dict keys is feasible, as some keys include hyphens, i.e. instance-id. Unless we change these keys, but that'll force changes in the SCFs of all users.
• Doesn't suppressing some keys or renaming them require a copy of the .__dict__? Otherwise the attributes themselves are being changed/removed. At that point I'd almost rather build up a valid representation of the input data dict from the stored attributes, rather than break down/modify a copy of .__dict__.
• SiteConfig sounds particularly tricky, since self.hub and self.hosts are HubConfig and HostConfig objects, respectively, so .__dict__ would produce something like:

  {'hub': HubConfig(<hub data dict repr>),
   'hosts': {'host1': HostConfig(<host data dict repr>),
             'host2': HostConfig(<host data dict repr>)}}
  

  But ideally we have it return a dict like the data input so that the repr would look like a valid Python expression that could be used to recreate a SiteConfig object with the same value.

[mhasself]

So ... yes, I think it's safe at this point to remove .from_dict and just make that the default constructor.

Great, how do you want me to handle that with respect to this PR? Include it? Separate? Before/After this one? (Hopefully after, since I've done that already on top of this branch.)

Whatever you like. Include is fine; later is fine.

Some thoughts on using .__dict__:

• I'm not sure changing attributes to match the dict keys is feasible, as some keys include hyphens, i.e. instance-id. Unless we change these keys, but that'll force changes in the SCFs of all users.

Agreed.

• Doesn't suppressing some keys or renaming them require a copy of the .__dict__? Otherwise the attributes themselves are being changed/removed. At that point I'd almost rather build up a valid representation of the input data dict from the stored attributes, rather than break down/modify a copy of .__dict__.

Yes. Constructing a valid data dict on the fly will be easier in cases that don't have many keys.

• SiteConfig sounds particularly tricky, since self.hub and self.hosts are HubConfig and HostConfig objects, respectively, so .__dict__ would produce something like:
  {'hub': HubConfig(<hub data dict repr>), 'hosts': {'host1': HostConfig(<host data dict repr>), 'host2': HostConfig(<host data dict repr>)}}
  But ideally we have it return a dict like the data input so that the repr would look like a valid Python expression that could be used to recreate a SiteConfig object with the same value.

That doesn't bother me much. I won't usually be trying to construct new SiteConfig objects from old reprs I have lying around. But honestly a super long repr like this is just going to annoy me most of the time. I would rather cache the input dict in _data, and if I need to access it, print(sc._data).

I'm don't feel super strongly about this... none of the solutions is totally satisfactory. Go ahead with what seems most useful to you.