Revise the way outputs are printed to the console

[matthewcarbone]

This has been causing me some headaches:

diffpy.pdffit2/diffpy/pdffit2/output.py

Line 28 in c4a2989

def redirect_stdout(dst):

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:

1. Simply use print, instead of redirecting stdout, allowing users to catch this in an outer context
2. Allow the user to disable this block when initializing PdfFit.

Thanks!

[sbillinge]

Thanks Matt, we will look into it. S

…

On Mon, May 13, 2024 at 12:04 PM Matthew Carbone ***@***.***> wrote: This has been causing me some headaches: https://github.com/diffpy/diffpy.pdffit2/blob/c4a2989a33f0ebaf0b650962073b0e04bec141de/diffpy/pdffit2/output.py#L28 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: 1. Simply use print, instead of redirecting stdout, allowing users to catch this in an outer context 2. Allow the user to disable this block when initializing PdfFit. Thanks! — Reply to this email directly, view it on GitHub <#25>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABAOWUNGNENQPFTQOB3KAETZCDQARAVCNFSM6AAAAABHUQALGCVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI4TGMRSHAYDQNQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

-- Simon Billinge Professor, Department of Applied Physics and Applied Mathematics Columbia University

[sbillinge]

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]

@8bitsam Could you please share any information that might could be helpful? We want to address this issue.

[8bitsam]

Yes, I think it would require a refactoring of the pypdffit2_redirect_stdout PyObject in misc.cc to 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.py with redirect_stdout() would also need to be modified to add functionality for the new argument.

[Tieqiong]

@8bitsam Any updates? Thanks!

@matthewcarbone Hello! Just to confirm, are you asking for:

1. An option to remove the copyright statement when creating a PdfFit object?
2. Confirmation that the default behavior uses sys.stdout for output?

Regarding the second point, it seems redirect_stdout() isn’t automatically invoked, so any output should already default to sys.stdout. Let me know if I misunderstood!

[matthewcarbone]

@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.py entirely, 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]

@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]

@8bitsam yes please. Thanks

[Tieqiong]

Note for future use: If the output cannot be captured, the safest approach is to use diffpy.pdffit2.output.stdout

[sbillinge]

Note for future use: If the output cannot be captured, the safest approach is to use diffpy.pdffit2.output.stdout

@Tieqiong can we add this comment somewhere in the pdffit2 docs?

[sbillinge]

Let's test this on the 1.5.0rc release.