Cleaning up main script

[ksuchak1990]

Hey @eeyouol and @nickmalleson - I've made some aesthetic changes to the main script for producing figures. The main changes that I've made here are:

1. Replacing print statements that keep track of loops with tqdm which introduces a progress bar;
2. Replacing most of the print statements with logging which provides a more standardised set of outputs;
3. Remove the if __name__ == "__main__": from the script - I believe that this script will only ever be run as a script and not imported as a module(?) so I think it makes sense to have it this way;
4. Linting the script to make the code style more consistent with pep8.

Points 1. and 2. have resulted in corresponding changes in the parallelised PF.

@eeyouol - It would be great if you could run the version that is on this cleanup branch before merging the pull request to make sure that the output figures haven't changed (I don't think that they have but would prefer to be sure).

[yannickoswald]

Hi @ksuchak1990 thanks very much for this! I really like the progress bars. I didn't know this package. nice!

I just tested the script and it works until logging info logging.info("Running Particle Filter (PF)"). I think the if name == "main": is necessary to run the parallelized particle filter in SPyder at least.

in ananconda prompt i get his error too and i think it repeats the entire script instead of just parallelizing the filter

    raise RuntimeError('''
RuntimeError:
        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__':
                freeze_support()
                ...

        The "freeze_support()" line can be omitted if the program
        is not going to be frozen to produce an executable.

[ksuchak1990]

It seems strange to me that the __name__ == "__main__" should be required... Will have a look at it now and see if I can reproduce the error on my machine.

[ksuchak1990]

in ananconda prompt i get his error too and i think it repeats the entire script instead of just parallelizing the filter

    raise RuntimeError('''
RuntimeError:
        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__':
                freeze_support()
                ...

        The "freeze_support()" line can be omitted if the program
        is not going to be frozen to produce an executable.

Ok, I've managed to reproduce this, so I guess the __name__=="__main__" is necessary. I'm going to have a quick play around and see if I can find get this to work.

I think the most reasonable solution will be to put everything inside the __name__=="__main__" (if seems a bit messy to have half of the script inside it and the other half outside it).

[ksuchak1990]

☝️ This works in anaconda prompt and spyder for me, I'll also test it out on my Linux terminal.

[yannickoswald]

Referen

Hi yes on Windows it is definitely compulsory. On Linux it might work without, but please let us keep in mind windows users. see here

https://stackoverflow.com/questions/20222534/python-multiprocessing-on-windows-if-name-main

i am not sure about the upper half in or out. I think i tested different versions and did it for a reason but cannot remember.

[ksuchak1990]

Referen

Hi yes on Windows it is definitely compulsory. On Linux it might work without, but please let us keep in mind windows users. see here

https://stackoverflow.com/questions/20222534/python-multiprocessing-on-windows-if-name-main

i am not sure about the upper half in or out. I think i tested different versions and did it for a reason but cannot remember.

Yep, trying to make it work for all platforms.

I think generally it's good practice to wrap everything in the __name__=="__main__". It should not make any difference to the first half.

[ksuchak1990]

I've just tested this on Linux too and it looks like it works there too so I've added the above couple of commits (https://github.com/eeyouol/covpol/pull/22/commits/85f7a389ca9e179cdebc256b0241118de2618489 and https://github.com/eeyouol/covpol/pull/22/commits/74a41ce77134aa2757478394a094dc8e65293628) to this PR.

Let me know if that works for you now.

[yannickoswald]

I've just tested this on Linux too and it looks like it works there too so I've added the above couple of commits (85f7a38 and 74a41ce) to this PR.

Let me know if that works for you now.

Perfect. that all works perfectly as well now. Thanks for doing this stuff. will approve the pull now.