Pytorch-lightning: Should the Trainer attributes be protected?

[awaelchli]

❓ Why are the Trainer attributes not protected?

Sorry if this was already discussed before, but I was wondering.
Currently, the following code works and doesn't complain:

trainer = Trainer()
trainer.checkpoint_callback = 'bad manners'

Is there a reason why attributes like these are not protected? For example this

class Trainer:
    def __init__(self):
        self._checkpoint_callback = ...

    @property
    def checkpoint_callback:
        return self._checkpoint_callback

would make the attribute _read only_ and prevent anyone from accidentally changing the state of The trainer.

Is the callback system possibly the reason for this, so that the callback method has full access to trainer variables? If so, I feel a property + setter approach would still be better.

[Borda]

I agree that some shall be protected as well as some methods which are not meant to be used by user shall be protected too...

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Evpok]

This is marked as wontfix so it probably won't be done, but for the record that would be a very unusual behavior for Python, “we are all consenting adults here” and I don't think that a lib going out of its way to do that kind of thing would be a good thing.

[awaelchli]

totally agree :)
the bot added this wontfix label and wanted to close the issue due to inactivity.
these robots... always trying to take over the world

[Borda]

This is marked as wontfix so it probably won't be done

sorry for the confusion, it was an automatic bot marking non-active issues...
Agree with @awaelchli we ll do it, just did not have time yet :rabbit:
@Evpok mind take it over and draft a PR with these changes?

[Evpok]

Well, I was trying to argue that it should not be done, since it adds unnecessary complexity and makes it harder to monkey patch if needed :-)

[Borda]

the point of protected is to clearly separate what is the recommended API and what is internal staff so you can still use it but to be aware that you are using something that is meant to be just internal PL use and you need to know how to use it correctly... so it does not add any complexity, all functions/classes stay as they are just minor rename with _

[Evpok]

Right, that makes sense, sorry for being dense :-)

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically marked as stale because it hasn't had any recent activity. This issue will be closed in 7 days if no further activity occurs. Thank you for your contributions, Pytorch Lightning Team!