joshspeagle / dynesty

Dynamic Nested Sampling package for computing Bayesian posteriors and evidences
https://dynesty.readthedocs.io/
MIT License
347 stars 76 forks source link

Please allow results() to be modified again, as in ver. <=1.1 #355

Closed 3fon3fonov closed 2 years ago

3fon3fonov commented 2 years ago

Before, Results() was a dict object and one could modify it. For example, one could add bi-products on the posteriors, add labels, and more.... Everything was stored in one place and accessible to the user at any time.

Forgive me if I don't understand why you had to break the API, and why now Results() should not be modified. Perhaps you had a reason, still, you should allow for some backward compatibility with versions <1.1 or some solution for old dynesty samplers saved on disk.

segasai commented 2 years ago

Obviously the choice of changing the Results API was a choice. I do think it is the right one and it helped to simplify a lot of logic in dynesty, but feedback is certainly welcome.

I certainly could remove the code that prohibits the modification of attributes, that's easy, but I think you just should not rely on modifying the Results object. If you want to do that, create a dictionary.

3fon3fonov commented 2 years ago

Thanks for your fast reply!

dict(results.items()) seems a good option, but what I did for backward compatibility was to "monkeyjack" the Results class with the old Results class, which I named dynesty_patch.py, and I left it in the Exo-striker /lib directory. Then:

import dynesty
import dynesty_patch
dynesty.results =  dynesty_patch

Did the job.

BTW, please push dynesty==1.2 to pypi !