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

Not friendly to non-ascii data #63

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

mih
Copy link
Contributor

@mih mih commented Mar 22, 2018

Russia is our friend!

from participants.tsv (valid UTF-8)

sub-01\tm\t30-35\tr\tn\tрусский

leads to

  File "/home/mih/hacking/PyNIDM/nidm/experiment/Core.py", line 332, in serializeJSONLD
    return rdf_graph_parse.serialize(format='json-ld', context=context, indent=4).decode('ASCII')

the decode part is the issue.

@mih
Copy link
Contributor Author

mih commented Mar 22, 2018

It forces it for turtle, but not for JSON-LD -- which can definitely be UTF-8.

@@ -91,7 +91,7 @@ def main(argv):
if args.jsonld:
f.write(project.serializeJSONLD())
else:
f.write(project.serializeTurtle())
f.write(project.serializeTurtle().decode('ASCII'))
Copy link
Contributor

Choose a reason for hiding this comment

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

pynidm is py3 only. why would this be necessary. were you testing on python 3 or 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was just moving the decode('ASCI') away -- not introducing it. I am not sure why one wants this, but it certainly is detrimental for JSON-LD. If you are confident that it being there at all (and not just in the way of JSON-LD) is the real bug, I have no objections to wipe it out entirely. And no, this is all PY3.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks - will take a look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants