RecursionError in LocalFileStorage about openff-evaluator HOT 2 CLOSED

My suggestion would be to just not privatize attributes with getter/setters, as I think it adds burden to code maintenance and is liable to bugs such as the one reported here. (The actual attribute used is _root_directory.)

This is a pretty common code pattern and gives a bit of safeguarding in case you later change the internal code to not be resilient to external changes of this variable - I think the main issue here is bad testing and not necessarily maintenance burden!

A PR containing the typo fix + test would be welcome 🙂

from openff-evaluator.

This is a pretty common code pattern and gives a bit of safeguarding in case you later change the internal code

Sure, I suppose I am a fervent follower of YAGNI and think it's fairly easy to change an attribute to a managed property -- if anything, this kind of code encourages me to use the "private" attribute because the public one can't really be used -- but we all have different styles :-)

I think the main issue here is bad testing and not necessarily maintenance burden!

I think of needing to write and maintain additional tests as maintenance burden!

from openff-evaluator.