add ifmain check to setup.py

[sbailey]

Context: I recently upgraded my laptop base conda installation to use python 3.9.12, which effectively broke unit tests started via "python setup.py test". I know the new procedure is to use pytest instead, but old habits die hard...

Symptom: The test_binscripts test of desispec.scripts.stdstars.main when called as a function (but not when called via a spawned script) would go crazy on the multiprocessing, and start running multiple versions of the test including multiple lines like "running build_ext", sometimes printing the message:

        An attempt has been made to start a new process before the
        current process has finished its bootstrapping phase.

        This probably means that you are not using fork to start your
        child processes and you have forgotten to use the proper idiom
        in the main module:

            if __name__ == '__main__':
...

See desihub/redrock#209 as well. The test would run fine if launched via pytest or by directly running python py/desispec/test/test_binscripts.py.

Solution: Add if __name__ == '__main__': guard to setup.py itself.

Help Wanted: does anyone know any potential negative side effects from this, e.g. interactions with pip? setup.py is a bit of a special thing, different from our standard bin/* scripts so I'd appreciate a second opinion on this. @weaverba137 @dkirkby or others?

[weaverba137]

Could you unpack the description a little bit more? Is the goal here to prevent someone from running python setup.py test? If so, that's a good thing.

More generally, it's not considered necessary to test a main() function itself. In fact, main() is automatically excluded when measuring test coverage.

Suppose you have a simple script:

#!/usr/bin/env python
from sys import exit
from package.subpackage import main
exit(main())

And package/subpackage.py:

def do_stuff():
    return 0

def main():
    status = do_stuff()
    return status

And package/test/test_subpackage.py:

from ..subpackage import do_stuff

def test_do_stuff():
    assert do_stuff() == 0

This test pattern should eliminate any need to hide things in if __name__ == '__main__':

[sbailey]

Thanks for taking a look at this.

Clarifying: The goal is to re-enable support for "python setup.py test" in case someone like me types that even if they were supposed to type "pytest" instead. The problem was that setup.py itself didn't have a if __name__ == '__main__': wrapper, and that was somehow confusing python 3.9.12 multiprocessing leading to tests gone wild.

Related: the symptom was the same as desihub/redrock#209 where rrdesi multiprocessing was broken on newer python until a if __name__ == '__main__': guard was added to bin/rrdesi.

[weaverba137]

I would just change my habits. You really, really need to get into the habit of not using python setup.py at all. I'm sorry, but that's the way the community is moving, and I don't think we should support deprecated habits.

[sbailey]

In the meantime, is there any actual harm to having an ifmain guard in setup.py?

[weaverba137]

I really don't know. All I can say is that I have never seen a setup.py file in any package set up that way.

[sbailey]

Thanks for the comments. Although it's non-standard to use if __name__ == "__main__": within a setup.py file, I also haven't found any reason why that would be disallowed, and it does help in this case to prevent python setup.py test from going wild with rogue parallelism so I'll merge this. If we post facto find that it breaks something, it's easy to take back out.

I'll try to remember to use pytest in the future, but there is one feature of "python setup.py test -m desispec.test.test_blah" that pytest doesn't seem to support: adding import IPython; IPython.embed() into a failing test to drop into a command line to inspect what is going wrong. So as long as setup.py isn't completely deprecated, I occasionally still fall back to that when debugging a failing test.

[Waelthus]

FYI: Using pytest one can do things like this pytest --pdb --pdbcls=IPython.terminal.debugger:TerminalPdb (the options can be set-up as defaults in $PYTEST_ADDOPTS) which should bring up an ipython powered debugger whereever a test fails. For me, that's typically even simpler than ingesting the statements into failing tests. Might work for your usecase as well.