-
Notifications
You must be signed in to change notification settings - Fork 192
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
Python3 support for restapi #2117
Conversation
select working version of PycCifRW for python2 and python3
…o write text instead of bytes.
_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
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.
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.
Also try to reduce the number of exclusions when possible, thanks!
aiida/restapi/resources.py
Outdated
id=id, | ||
query_string=request.query_string, | ||
id=node_id, | ||
query_string=query_string, |
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.
Before it was unquoted. Was it wrong before or now? Note there is another place in the code which has a similar pattern, good to check what is done there.
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.
Unquoted and quoted query string in response object both works in same way.
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.
there is one common anti-pattern here:
def func(something=None):
if something is None:
something = "some default value"
pass
where I would do instead:
def func(something="some default value"):
pass
for k, v in headers.items(): | ||
response.headers[k] = v | ||
for key, val in headers.items(): | ||
response.headers[key] = val |
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.
for next time, possibly use respone.headers.update(headers)
as a shorthand
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.
Here response.header is not a type of dict so we can not directly update it with another dictionary.
|
||
@staticmethod | ||
def get_visualization_data(node, format=None): | ||
def get_visualization_data(node, visformat=None): |
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.
why not put visformat="xsf"
here directly? then its clear by looking at the signature of the function what the default is
@staticmethod | ||
def get_downloadable_data(node, format=None): | ||
def get_downloadable_data(node, download_format=None): |
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.
... same here
This fixes #2086 |
No description provided.