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

better JSON munging or workaround #7

Closed
shabbychef opened this issue Dec 24, 2012 · 12 comments
Closed

better JSON munging or workaround #7

shabbychef opened this issue Dec 24, 2012 · 12 comments

Comments

@shabbychef
Copy link
Contributor

The following will choke pymatbridge:

%%matlab
fprintf('got quotes? -->  "    ');

The JSON parser gets confused, of course. We can work on escaping and unescaping the I/O to and from Matlab, but it might make more sense to use temporary files (blech), or another mechanism, to move text around.

@arokem
Copy link
Owner

arokem commented Dec 24, 2012

Alternatively, we can keep adding stuff to this (ugly) part of the code, as
they come up:

https://github.com/arokem/python-matlab-bridge/blob/master/pymatbridge/pymatbridge.py#L187

On Mon, Dec 24, 2012 at 1:43 PM, Steven Pav notifications@github.comwrote:

The following will choke pymatbridge:

%%matlab
fprintf('got quotes? --> " ');

The JSON parser gets confused, of course. We can work on escaping and
unescaping the I/O to and from Matlab, but it might make more sense to use
temporary files (blech), or another mechanism, to move text around.


Reply to this email directly or view it on GitHubhttps://github.com//issues/7.

@shabbychef
Copy link
Contributor Author

BSON would be another possibility; we could crib this code for the Matlab end. It might be a bit bumpy, though.

@shabbychef
Copy link
Contributor Author

hmm, so the mat2json code is very happy to produce JSON that JSON lint barfs on. There would have to be some fixes in around this line, which I am happy to implement. There's no sense, in my opinion, in having a mat2json which does not adhere to the JSON standard (I realize this code was taken from the Mathworks 'file exchange', so upstream fixes may exist somewhere, or it may be orphaned.)

@arokem
Copy link
Owner

arokem commented Jan 3, 2013

Sorry - dropped that ball over the holiday.

Just to continue this discussion - it doesn't surprise me too much - I was
suspecting the json implementation was not complete. I am not sure how to
handle this. One option is to fix the implementation we have there. If you
want to do that, that would be great. Another option is to switch into a
(more? completely?) complete implementation of json. There are a few
options.

This seems to be the most up-to-date one:
http://www.mathworks.com/matlabcentral/fileexchange/33381-jsonlab-a-toolbox-to-encodedecode-json-files-in-matlaboctave

This one is slightly newer (most recent commit is about 5 months ag), but
has a mex dependency, which is always a PITA:
https://github.com/christianpanton/matlab-json

Any thoughts on how to do that? We could set that as an external dependency
and then remove all the json-type code in our code-base and call out to
this instead, or copy their code-base into our repository, or...?

On Wed, Dec 26, 2012 at 11:52 AM, Steven Pav notifications@github.comwrote:

hmm, so the mat2json code is very happy to produce JSON that JSON linthttp://jsonlint.combarfs on. There would have to be some fixes in around this
linehttps://github.com/arokem/python-matlab-bridge/blob/master/pymatbridge/matlab/functions/mat2json.m#L57,
which I am happy to implement. There's no sense, in my opinion, in having a
mat2json which does not adhere to the JSON standard (I realize this code
was taken from the Mathworks 'file exchange', so upstream fixes may exist
somewhere, or it may be orphaned.)


Reply to this email directly or view it on GitHubhttps://github.com//issues/7#issuecomment-11693040.

@shabbychef
Copy link
Contributor Author

not a prob;

I implemented a very simple fix, and have yet to break it in my testing. The corresponding issue lurking in json2mat does not seem to have bit me yet, which I find odd.

@arokem
Copy link
Owner

arokem commented Jan 13, 2013

Hey - I just merged some fixes from someone and noticed that you have been
moving ahead on your fork with some good stuff. Would it be a good time to
merge your commits into mine, before things diverge very far, or where you
planning to put in a PR against my master at some point?

On Thu, Jan 3, 2013 at 2:43 PM, Steven Pav notifications@github.com wrote:

not a prob;

I implemented a very simple fixhttps://github.com/shabbychef/python-matlab-bridge/blob/master/pymatbridge/matlab/functions/mat2json.m#L57,
and have yet to break it in my testing. The corresponding issue lurking in
json2mat does not seem to have bit me yet, which I find odd.


Reply to this email directly or view it on GitHubhttps://github.com//issues/7#issuecomment-11864458.

@shabbychef
Copy link
Contributor Author

hi;
I'm actually a bit of a n00b on DVCS and am a little unclear on the mechanics and the etiquette. I also wanted to make sure I hadn't broken anything before submitting a PR, and have no experience with unit testing in Python. (And there is no commonly accepted testing framework in Matlab, FWIW.)
Modulo all that, is there an accepted 'master' branch on this project? Yours or @jaderberg 's? Where should I submit issues? I've started putting them in my own branch so I don't lose track, but this is obviously suboptimal.

cheers,

@arokem
Copy link
Owner

arokem commented Jan 14, 2013

Hi,

I am also rather new to this kind of situation and I admit that I hadn't
given this much thought and maybe now would be good time to decide on these
issues, as I started to get PRs from one more person (@jenshnielsen).

Honestly, when I started hacking on the fork I made off @jaderberg's
original repo, I didn't know whether this was a blind alley, or something
that I would want to keep using and working on, so I didn't submit PRs
against his repo, but maybe I should have. I don't mind merging all the
more recent development (the ipython stuff, changes to the original module
structure, and the improvements you have made) into @jaderberg's original
repository and we can switch to merging upstream back to his repo, if he
wants to integrate back changes, as we keep working on this. On the other
hand, I don't mind giving other people commit rights on my fork. Either
way, it would make sense for more people to have commit rights on whatever
we decide to have as the 'one true upstream', so that we don't diverge too
much, on the one hand, and so that more people can review PRs and push into
that upstream.

Max - do you have any strong opinion either way? I think that it would make
sense to merge everything into your repo, assuming you want all these
recent developments there. I'm sorry that things have diverged as far as
they have already

On Mon, Jan 14, 2013 at 10:03 AM, Steven Pav notifications@github.comwrote:

hi;
I'm actually a bit of a n00b on DVCS and am a little unclear on the
mechanics and the etiquette. I also wanted to make sure I hadn't broken
anything before submitting a PR, and have no experience with unit testing
in Python. (And there is no commonly accepted testing framework in Matlab,
FWIW.)
Modulo all that, is there an accepted 'master' branch on this project?
Yours or @jaderberg https://github.com/jaderberg 's? Where should I
submit issues? I've started putting them in my own branch so I don't lose
track, but this is obviously suboptimal.

cheers,


Reply to this email directly or view it on GitHubhttps://github.com//issues/7#issuecomment-12231093.

@jaderberg
Copy link
Contributor

I'm not actually that familiar with Github so don't really know how to handle all this! I reckon you should just use your repo @arokem - you've dont a lot of work on it and I am not going to be touching the project in the near future. I don't actually know how I would merge things in anyway... Sorry for being useless!

@shabbychef
Copy link
Contributor Author

ok, it looks like @arokem 's repo is now the master. I'll file my issues there and try to keep up PRs. (sorry about the CRLF nonsense. I'll see if I can straighten it out.)

@arokem
Copy link
Owner

arokem commented Jan 16, 2013

Great. Hopefully we can keep making this better!

On Tue, Jan 15, 2013 at 8:17 PM, Steven Pav notifications@github.comwrote:

ok, it looks like @arokem https://github.com/arokem 's repo is now the
master. I'll file my issues there and try to keep up PRs. (sorry about the
CRLF nonsense. I'll see if I can straighten it out.)


Reply to this email directly or view it on GitHubhttps://github.com//issues/7#issuecomment-12304179.

@blink1073
Copy link
Collaborator

Works on current master, closing as fixed.

In [6]: %%matlab
   ...: fprintf('got quotes? -->  "    ');
   ...: 
got quotes? -->  "    

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

No branches or pull requests

4 participants