Nexus: Move all log() functions to nxs_print() for future use of logging library - #6209
Open
brockdyer03 wants to merge 3 commits into
Open
Nexus: Move all log() functions to nxs_print() for future use of logging library#6209brockdyer03 wants to merge 3 commits into
log() functions to nxs_print() for future use of logging library#6209brockdyer03 wants to merge 3 commits into
Conversation
Contributor
|
Please add a description of the long standing issue that you fixed in progress_tty. This actually worked for me, so I am wondering what the issue was. |
Contributor
Author
I've updated the main post with a description of the fix. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
This PR renames the
log()function ingeneric.pytonxs_print(), renamesDevBaseNexus.log()toDevBaseNexus.nxs_print(), and renamesNexusCore.log()toNexusCore.nxs_print(). It also removesgeneric_settings.devlog(andgeneric_settingsas a whole sincedevlogwas the last thing in it), and replaces any mention of it withsys.stdout.Additionally, I fixed a long-standing issue with the
progress_ttysetting in Nexus, which attempted to overwrite the previous line that was written for polling prints so that the polls would not all be on their own line. The previous method used a fixed amount of whitespace before the carriage return to try and ensure that the entire previous line was cleared before the next line was printed, but would leave dangling text if the next line was significantly shorter and did not cover enough space to overwrite the previous line. I used the ANSI sequence for line up ("\033[1A") and"\033[2K"to clear the whole line. This is a much more robust way to ensure that the previous line is cleared. (seeNexusCore.nxs_print())As a side note, when testing the fix to
progress_ttymode, I found the output it produced to be significantly nicer than the regular output of Nexus, and I think that we should probably switch the default behavior to be that ofprogress_tty.Motivation
The Nexus "log" function has long behaved just like a custom print function and has not really been a true logger. In the hopes of eventually using Python's
logginglibrary, I have decided to move all of that framework to the namenxs_print, which is more in line with what it actually is for.Additionally, I removed
generic_settings.devlogfor a few reasons. Number one is that it was never particularly clear if/when it should change from beingsys.stdout, and number two is that it further simplifies the relationship between parts of Nexus.Testing changes
In removing
generic_settings.devlog, I also removed the method by which Nexus's test system captured any writes tosys.stdout, which was through theFakeLogclass innexus/tests/__init__.py. This was replaced by the Pytest fixturecapsys, which capturesstdoutandstderr, then clears the buffers, essentially performing the same task asFakeLog, except using the standard method that the broader Python/Pytest community uses (which I know @prckent will appreciate).What type(s) of changes does this code introduce?
Does this introduce a breaking change?
What systems has this change been tested on?
Laptop, Fedora Linux 44 (KDE Plasma Desktop Edition)
AMD Ryzen 7 PRO 7840U (8 cores, 16 logical processors)
Checklist