Don't call render_targets() from get_targets() when all requested targets are cached

[simu]

Proposed Changes

Only render Reclass inventory if render_targets() is called with either targets=None or a non-empty list of targets to render. Without this, the whole Reclass inventory is rendered for pretty much every call to get_targets() since that method calls render_targets() with targets=[] if it already has parameters for all targets.

Docs and Tests

• Tests added
• Updated documentation

[MatteoVoges]

The idea of the targets list was to give the backend control about the strategy for rendering specified targets. As reclass has no such concept ( afaik it has single target and everything) I thought we would render everything regardless, which targets are specified or even if any targets are specified.

Maybe we should remove the default value None for targets, so that the logic of not rendering with empty targets can live at a higher level.

I just wanted to move away from the behavior empty list = all targets, to make it more explicit.

[simu]

I just wanted to move away from the behavior empty list = all targets, to make it more explicit.

That's fine, but the ReclassInventory backend doesn't actually respect empty list == no targets without this PR

[simu]

As reclass has no such concept ( afaik it has single target and everything) I thought we would render everything regardless, which targets are specified or even if any targets are specified.

If you want to selectively render targets ("nodes" in reclass terminology), you could use nodeinfo()

[MatteoVoges]

That's fine, but the ReclassInventory backend doesn't actually respect empty list == no targets without this PR

I see, what about just adding an if targets_to_render: inside the get_targets() method?
Because I still think, this should be a general and not backend specific thing.

[simu]

That's fine, but the ReclassInventory backend doesn't actually respect empty list == no targets without this PR

I see, what about just adding an if targets_to_render: inside the get_targets() method? Because I still think, this should be a general and not backend specific thing.

I assumed that render_targets(targets=[]) probably works as expected (i.e. doesn't render anything) with OmegaConf, so I thought I'd just fix it in the Reclass backend. I'm happy to adjust get_targets() to not even call render_targets() if it decides that nothing needs to be rendered.

[MatteoVoges]

Thanks for the update!