Skip to content

Conversation

StoicLoofah
Copy link
Collaborator

This addressed issue #63.

Looking into the issue, it appears that python3 handles format strings differently from python2. In this case, both None and not None values were failing because neither had properly implemented __format__ necessary for the alignment formatting provided. The easy fix is to cast to a string first, and that can be properly formatted.

I considered adding __format__ to Entity, but I saw there was already something in there and got scared.

def format(self, format_string):

Maybe someone else will get around to it if this same issue comes up in a another place. At least this fix appears to work for now

@StoicLoofah
Copy link
Collaborator Author

@miguelgondu let me know if this is working for you in the general case

StoicLoofah and others added 2 commits October 31, 2018 02:38
Load a lotv replay and print all the events to ensure they print.
The output from the prints have been redirected to a stream using
StringIO and then checked for the last few events to occur.
@Gusgus01
Copy link
Contributor

@StoicLoofah I think I have updated and tested this change in: StoicLoofah#3 which if you accept you can then update this pull request I think?

There was a rebase involved to bring it up to date with upstream to avoid a merge commit, which I haven't had to do before and might not work.

StoicLoofah and others added 2 commits November 1, 2018 14:25
Logic issue with conditional import.
StringIO.StringIO does not support usage of the with keyword.
@Gusgus01
Copy link
Contributor

Gusgus01 commented Nov 2, 2018

StoicLoofah#4

At this point with all the extraneous commits, it might be worth squashing the commits when merging into ggtracker:upstream. Or maybe pulling your branch down and squashing into two commits if you want to keep the "authors" correct. :)

Fix Python 2 issues with test_event_print.
@StoicLoofah
Copy link
Collaborator Author

@Gusgus01 thanks for the tip. I'm not too concerned about the exact commit history, so I will lazily merge as is. thanks for helping to validate and test this fix!

@StoicLoofah StoicLoofah merged commit b21d45e into ggtracker:upstream Nov 4, 2018
@StoicLoofah StoicLoofah deleted the 63-trackerevent_str_error branch November 4, 2018 18:25
print(event)
self.assertIn("PlayerLeaveEvent", capturedOutput.getvalue())
sys.stdout = sys.__stdout__
sys.stdout = capturedOutput = StringIO()
Copy link
Collaborator

@cclauss cclauss Nov 4, 2018

Choose a reason for hiding this comment

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

The original with open() as form is safer in the face of exceptions... Was there a reason to move away from that?

Copy link
Contributor

Choose a reason for hiding this comment

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

StringIO.StringIO does not implement exit() I believe and can not be used with with() in python2. io.StringIO does and is what is being used in Python3 so could be used with with().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants