Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix import of fancylogger with Python 3 #283

Closed
wants to merge 4 commits into from

Conversation

boegel
Copy link
Member

@boegel boegel commented Jul 5, 2019

Baby step towards supporting Python 3, I apparently had this sitting in a branch but never opened a PR for it (I knew #282 looked familiar).

More changes are needed to actually run the fancylogger tests, that would be the next goal I guess...

@@ -412,7 +418,7 @@ def somefunction(self):
handler = fancylogger.logToScreen()
logger = fancylogger.getLogger(fname=False, clsname=False)
logger.warn("blabla")
print stringfile.getvalue()
print(stringfile.getvalue())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should import from future ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import is there now (but this change is still needed, print 'foo' never works in Python 3, even with the import

set -e;
export PYTHONPATH="$PWD/lib:$PYTHONPATH";
python -c "import vsc.utils.fancylogger";
python -c "import test.fancylogger";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not actually test anything, so why bother?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It saves some of the problems, makes the PR short and easy to review, etc., the tests pass as is, etc.

Baby steps.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify:

  • this checks whether vsc/utils/fancylogger.py no longer has SyntaxErrors; same for test/fancylogger.py
  • can't run only the fancylogger subsuite yet, more changes are needed for that due to SyntaxError in test/missing.py

If you prefer, I can fix the import test/missing.py too, and see if that's enough to run the fancylogger tests with Python 3 (but it's going to blow up the PR, why not merge it first since it's easy to review right now).

@boegel
Copy link
Member Author

boegel commented Mar 12, 2020

Closing this in favor of #287

@boegel boegel closed this Mar 12, 2020
@boegel boegel deleted the py3_fancylogger_import_fixes branch March 12, 2020 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants