lmmentel / mendeleev

A python package for accessing various properties of elements, ions and isotopes in the periodic table of elements.
https://mendeleev.readthedocs.io
MIT License
220 stars 40 forks source link

ImportErrors are not thrown for missing objects #149

Closed lmmentel closed 4 months ago

lmmentel commented 4 months ago

Describe the bug

Spotted by @kalvdans in this comment.

A recent fix #148 introduced the mechanism by which when importing arbitrary names from mendeleev won't throw an ImportError

To Reproduce

Run

# whatever does not exist in the package
from mendeleev import whatever 

Expected behavior

Import Errors are thrown for non-existing objects

Screenshot

If applicable, add screenshots to help explain your problem.

Specification

lmmentel commented 4 months ago

@kalvdans do you have any suggestions on how to tackle this?

My initial thought is to delegate import responsibilities back to getattr somewhere at the end of overridden __getattr__ function in the mendeleev.__init__.py. Not entirely sure how at this point.

kalvdans commented 4 months ago

@kalvdans do you have any suggestions on how to tackle this?

I already gave my advice in https://github.com/lmmentel/mendeleev/pull/121#issuecomment-1612460655 to not mix static and dynamic content in the same namespace. My only advice if you still insist, is to write some unit tests for the behaviour you want, like

def test_nonexisting():
    with pytest.raises(AttributeError):
        _ = mendeleev.Unobtanium
lmmentel commented 4 months ago

Thanks! Have you thought about an alternative for the current way it's implemented? I would very much like to keep the shorthand import of elements by symbol without taking too much of a performance hit. BTW this has been available in mendeleev since v0.4.0.

Good point on the tests, I've added a few to reduce the risk of tripping over similar issues in the future.