I2PC / scipion

Scipion is an image processing framework to obtain 3D models of macromolecular complexes using Electron Microscopy (3DEM)
http://scipion.i2pc.es
Other
76 stars 47 forks source link

Refactoring proposal for ProgressBar class, fixed stdout mystery #2005

Closed delarosatrevin closed 5 years ago

delarosatrevin commented 5 years ago

Dear @rmarabini , as my review of PR #1998 ...I'm proposing some re-factoring to your ProgressBar class. I didn't want to directly commit my changes into your branch, so I have created this other PR to discuss my proposal.

Regarding the refactoring...I had two main concerns:

Regarding the mystery of stdout redirection...the issue was in your code. There is a STRONG recommendation in Python to not use mutable objects (e.g { }, [ ], etc) as default values of function...because this can introduce very weird issues if you modify the default value. This was the case here...you passed as default sys.stdout, but this was evaluated before the Protocol redirection of sys.stdout took place. The solution is easy once you have found the problem...use None as default and then inside the constructor, use sys.stdout if no other output was passed as argument.

rmarabini commented 5 years ago

I aprove this pull request and I will check it in my own pull request (

rmarabini commented 5 years ago

PLease,

do not confirm this merge, I will do it myself after I double checked it