Skip to content
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

Fix for #406 (ascii issue) #536

Merged
merged 10 commits into from
Nov 25, 2016
Merged

Fix for #406 (ascii issue) #536

merged 10 commits into from
Nov 25, 2016

Conversation

hakonsbm
Copy link
Contributor

@hakonsbm hakonsbm commented Nov 1, 2016

PyNEST will now raise a NESTError when trying to use non-ascii characters under Python 2.7.

We did not manage to find a way to make PyNEST work with non-ascii characters under Python 2.7. Note that this works under Python 3.

@mention-bot
Copy link

@hakonsbm, thanks for your PR! By analyzing the history of the files in this pull request, we identified @otizonaizit, @apeyser and @jougs to be potential reviewers.

@jougs
Copy link
Contributor

jougs commented Nov 2, 2016

@hakonsbm: can you please remove the trailing whitespace in pynest/pynestkernel.pyx? Thanks!

Copy link
Contributor

@jougs jougs left a comment

Choose a reason for hiding this comment

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

LGTM.

@heplesser heplesser added ZC: PyNEST DO NOT USE THIS LABEL I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) ZP: PR Created DO NOT USE THIS LABEL S: Normal Handle this with default priority T: Bug Wrong statements in the code or documentation labels Nov 17, 2016
@heplesser heplesser added this to the NEST 2.12 milestone Nov 17, 2016
@heplesser
Copy link
Contributor

@apeyser Could you have a look at this one and see if it is ready for a second thumb up?

@apeyser
Copy link
Contributor

apeyser commented Nov 17, 2016

I'd suggest wrapping up

if python3_test:
    def encode(s): return s.encode('utf-8')
    def decode(s): return s.decode('utf-8')
else:
    ... for python2.7

just to remove the repeated ifs scattered through the code.

@hakonsbm
Copy link
Contributor Author

@apeyser Something like this?

Copy link
Contributor

@apeyser apeyser left a comment

Choose a reason for hiding this comment

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

Just fix up the spacing and approved.


def decode(s): return s.decode('utf-8')
def decode(s):
return s.decode('utf-8')
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a newline after this

@@ -82,19 +82,21 @@ def catching_sli_run(cmd):
"""

if sys.version_info >= (3,):
engine.run('{%s} runprotected' % cmd) # Python 3
def encode(s): return s
Copy link
Contributor

Choose a reason for hiding this comment

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

Precisely like this. The rest of the code is simplified and clear.

@hakonsbm
Copy link
Contributor Author

@apeyser Fixed.

@apeyser apeyser merged commit 870d23b into nest:master Nov 25, 2016
@hakonsbm hakonsbm deleted the non_ascii_handling branch November 25, 2016 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Bug Wrong statements in the code or documentation ZC: PyNEST DO NOT USE THIS LABEL ZP: PR Created DO NOT USE THIS LABEL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants