-
Notifications
You must be signed in to change notification settings - Fork 87
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
Re-implement the 1/3 hex asciimap reader #49
Conversation
The old one was not interpreting the leading hyphens properly. The new ones is a more reliable inversion of the writing process, and handles spacing between elements better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with comments.
armi/utils/asciimaps.py
Outdated
@@ -69,6 +70,8 @@ def readMap(self, text): | |||
for col, char in enumerate(line.split()): | |||
i, j = self._getIndices(base, col) | |||
self.lattice[i, j] = char | |||
|
|||
self.lattice = {k: v for k, v in self.lattice.items() if v != "-"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should probably constantize this with
PLACEHOLDER = "-"
and then refer to PLACEHOLDER
here.
grid = {} | ||
# The length of the largest string contents. Useful for spacing everything out | ||
# nicely | ||
maxSize = len(str(next(iter(self.lattice.values())))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
groan is that really what we have to do to get the length of the first value!? Python 3 whyyyy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I had a private little hissy of my own when I found out that the various dict view classes arent themselves iterators. It feels like the iter()
call should be unnecessary.
armi/utils/asciimaps.py
Outdated
# of our otherwise infinite loop | ||
break | ||
if contents is None: | ||
contents = '-' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PLACEHOLDER
The old one was not interpreting the leading hyphens properly. The new
ones is a more reliable inversion of the writing process, and handles
spacing between elements better.