search

alchemistry / alchemlyb

the simple alchemistry library
https://alchemlyb.readthedocs.io
BSD 3-Clause "New" or "Revised" License
197 stars 51 forks source link

Why test with exception in test_import? #241

Closed DrDomenicoMarson closed 2 years ago

DrDomenicoMarson commented 2 years ago

I was checking for coverage and I saw this test 

import alchemlyb 

def test_name(): 
    try: 
        assert alchemlyb.__name__ == 'alchemlyb' 
    except Exception as e: 
        raise e

Why are we catching an Exception here in a test?

xiki-tempula commented 2 years ago

No idea. It seems to me that assert alchemlyb.__name__ == 'alchemlyb' should be enough. Could you use blame to check who wrote that part?

DrDomenicoMarson commented 2 years ago

I didn't know about "blame" :-)

It seems @ianmkenney did the commit 6 years ago! Could we just remove the try?

ianmkenney commented 2 years ago

I couldn't tell you why I thought that was either necessary or useful. Feel free to change!

DrDomenicoMarson commented 2 years ago

Great, I implemented the change in PR #240, I hope it's fine, just not to have another PR just for that!

orbeckst commented 2 years ago

If you had done another PR, I would have just merged it.

Micro-PRs are easier to maintain.