Skip to content
This repository has been archived by the owner on Jul 7, 2023. It is now read-only.

Slightly simplify python loops and attr access #1446

Merged
merged 1 commit into from
Feb 11, 2019

Conversation

lgeiger
Copy link
Contributor

@lgeiger lgeiger commented Feb 9, 2019

Just some minor readability improvements that accumulated during debugging bugs in my custom model code.

Just some minor readability imrpovements that accumulated during debugging bugs in my custom model code.
@googlebot googlebot added the cla: yes PR author has signed CLA label Feb 9, 2019
for i in range(len(self._id_to_token)):
f.write(self._id_to_token[i] + "\n")
for token in self._id_to_token:
f.write(token + "\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

this will not work, _id_to_token is a dict and the previous code basically writes tghe vocab file in order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I missed the fact that dicts aren't order prior to Python 3.6.

Copy link
Contributor

@afrozenator afrozenator left a comment

Choose a reason for hiding this comment

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

I added a comment, will revert that change on my end, you can also delete it from this PR if you wish.

@afrozenator
Copy link
Contributor

Thanks a lot again for this @lgeiger , I'm merging in now, will do that revert on my end.

@afrozenator afrozenator merged commit 928c62f into tensorflow:master Feb 11, 2019
@lgeiger lgeiger deleted the refactor branch February 11, 2019 18:53
tensorflow-copybara pushed a commit that referenced this pull request Feb 11, 2019
PiperOrigin-RevId: 233438134
kpe pushed a commit to kpe/tensor2tensor that referenced this pull request Mar 2, 2019
Just some minor readability imrpovements that accumulated during debugging bugs in my custom model code.
kpe pushed a commit to kpe/tensor2tensor that referenced this pull request Mar 2, 2019
PiperOrigin-RevId: 233438134
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has signed CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants