search

joshspeagle / dynesty

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

ImportError: cannot import name 'reflect' from 'dynesty.utils' #361

Closed ahnitz closed 2 years ago

ahnitz commented 2 years ago

Describe the bug I'll preface this that this is most likely not a bug, but rather a case of user error when using a function not intended to be used downstream. However, in the case that this affects others or was unintentional change, I thought it useful to raise.

In the latest of release of Dynesty, the 'reflect' function in dynesty.utils has been renamed 'apply_reflect'. Downstream codes might use this and other functions in this module if they want to implement a custom sample method for example.

Setup dynesty 1.2 [released March 31, 2022]

Bug

from dynesty.utils import reflect

produces 

ImportError: cannot import name 'reflect' from 'dynesty.utils'
ahnitz commented 2 years ago

A simple fix for downstream codes is try / except. The functionality seems unaffected between releases e.g. we are proposing for our own cod the following. 

    try:
        # dynesty <= 1.1
        from dynesty.utils import unitcheck, reflect
    except:
        # dynest >= 1.2
        from dynesty.utils import unitcheck, apply_reflect as reflect
ahnitz commented 2 years ago

The reason for the rename seems quite clear, see https://github.com/joshspeagle/dynesty/commit/d63ed50c72bc0c90d210b6843fb1427bbb918071

segasai commented 2 years ago

Thanks for the report. The reason for the renaming, was that there were function arguments in the code called reflect, so there was a risk of a name clash. But to be honest, I think those functions are not expected to be publically used (or at least I don't think we should guarantee API stability there)

segasai commented 2 years ago

I'll close the issue for now, as I think there is no action needed here.