Repository navigation
Revise the way outputs are printed to the console #25
Description
This has been causing me some headaches:
diffpy.pdffit2/diffpy/pdffit2/output.py
Line 28 in c4a2989
Every time PdfFit is instantiated, there is a block of text printed to the console. This similarly happens in other situations. I cannot catch this output because within the PdfFit.__init__ there is another line that globally overwrites stdout, making it impossible for me to redirect using e.g. a context manager. I'm not sure this is good practice: is there any particular reason this is done? I understand that it's important to the authors to display the copyright statements and whatnot, but surely it's reasonable to allow the user to disable this?
I have two suggestions:
- Simply use print, instead of redirecting stdout, allowing users to catch this in an outer context
- Allow the user to disable this block when initializing
PdfFit.
Thanks!
Activity
sbillinge commentedon May 14, 2024
sbillinge commentedon Jul 31, 2024
Note to selves, consult with @8bitsam when we turn to this. He played with this a bit and i think found issues in resolving it.
bobleesj commentedon Dec 5, 2024
@8bitsam Could you please share any information that might could be helpful? We want to address this issue.
8bitsam commentedon Dec 6, 2024
Yes, I think it would require a refactoring of the
pypdffit2_redirect_stdoutPyObjectinmisc.ccto redirect the same way it does now as a default, but allow for the user to also specify another stream to redirect to as a new (optional) argument. The default being the same as it is now is necessary to ensure other users' code doesn't break.After modifying the C++, the Python code in
output.pywithredirect_stdout()would also need to be modified to add functionality for the new argument.Tieqiong commentedon Jan 9, 2025
@8bitsam Any updates? Thanks!
@matthewcarbone Hello! Just to confirm, are you asking for:
Regarding the second point, it seems
redirect_stdout()isn’t automatically invoked, so any output should already default tosys.stdout. Let me know if I misunderstood!matthewcarbone commentedon Jan 9, 2025
@Tieqiong it's been a long time so I don't quite remember. I think though that you're right,
redirect_stdout()does not appear to be called anywhere. I don't recall if it was when I initially opened the issue.Anyways, I think this can be easily resolved by just getting rid of all of
output.pyentirely, and just printing normally here (I have no idea if there's actually anything wrong anymore or not, but this should definitely resolve the issue). Alternatively, just add a parameter that the user can change that disables that printing.8bitsam commentedon Jan 9, 2025
@Tieqiong I think I agree with @matthewcarbone, we could just add an option in the Python code to not print if we want. I never really touched this issue since I was weary about messing with the C++ code. @Tieqiong if you think just modifying the Python code is fine, let me know and I can put out a PR.
Tieqiong commentedon Jan 9, 2025
@8bitsam yes please. Thanks
Tieqiong commentedon Jan 11, 2025
Note for future use: If the output cannot be captured, the safest approach is to use
diffpy.pdffit2.output.stdoutsbillinge commentedon Jan 12, 2025
@Tieqiong can we add this comment somewhere in the pdffit2 docs?
sbillinge commentedon Jan 12, 2025
Let's test this on the 1.5.0rc release.