Fix CifData and tcod exporters for python3#2092
Merged
Merged
Conversation
select working version of PycCifRW for python2 and python3
_exportstring should return bytes. to reflect this, we are renaming it and updating its docstring
errors were only converted to test failures (the interactive part of the test is failing now)
still to do: fix base64 conversion
ltalirz
changed the title
Functions affected: 1) test_contents_encoding_1() 2) cif_encode_contents 3) encode_textfield_quoted_printable
…ke them runnable both on Python 2 and Python 3.
…ded to Unicode as PyCIFRW is not able to process bytes.
+ yapf fix
…erministic output in TcodExporter. Enabling one more Python 3 test case.
… expressions to avert problems caused by numeric precision.
…esults have to be decoded before usage.
encode_textfield_ncr and decode_textfield_ncr are not working properly yet, needs more investigation. skip test_contents_encoding_2 test for the moment
…abling the last test case for TcodExporter.
merkys
changed the title
Member
|
Work finished. @giovannipizzi you may review the code now. |
giovannipizzi
requested changes
Oct 24, 2018
| h = "0{}".format(h) | ||
| return "{}={}{}".format(prefix, h, postfix) | ||
| h = "0%s" % h | ||
| return b"%s=%s%s" % (prefix, h.encode('utf-8'), postfix) |
Member
There was a problem hiding this comment.
Is there a reason to use % instead of .format()? In all the rest of the code we use .format()
Member
There was a problem hiding this comment.
(Also for this, same question in many points)
Member
There was a problem hiding this comment.
@yakutovicha, have you changed these lines? I am actually not aware of the differences between % and format.
Member
There was a problem hiding this comment.
Apparently the .format() is not able to return bytes.
Member
There was a problem hiding this comment.
(Also for this, same question in many points)
This seems to be the last occurrence now, I have replaced others.
merkys
reviewed
Oct 24, 2018
merkys
reviewed
Oct 24, 2018
…NWChem plugin with skipIf.
…tring representation to simple dictionary comparison.
… functions of TcodExporter, therefore removing them.
giovannipizzi
requested changes
Oct 25, 2018
Member
giovannipizzi
left a comment
giovannipizzi
left a comment
There was a problem hiding this comment.
Just a final change, the rest is fine!
giovannipizzi
approved these changes
Oct 25, 2018
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
5 participants
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.
setup_requirements.pyfor python2 and python3