[sage-devel] Re: Bug report: Incorrect json representation for graphics.

2017-01-25 Thread mjsoto
Thanks for taking care of this. I see that you fixed the doctests too. 

However, doctests will also fail in the description of `json_repr` in file 
`base.pyx`, in a few instances.

More importantly, you absolutely should verify that you can send the 
corrected json to jmol and have it work.



El miércoles, 25 de enero de 2017, 11:14:56 (UTC+1), Frédéric Chapoton 
escribió:
>
> I have created a ticket on trac (
> https://trac.sagemath.org/ticket/22253#ticket)
>
> we do not use github and pulls, but trac tickets.
>
> I am going to post a branch there soon.
>
> Frederic
>
> Le mercredi 25 janvier 2017 11:12:11 UTC+1, mjs...@gmail.com a écrit :
>>
>> Markdown below: 
>>
>> Bug report: Incorrect json representation for graphics.
>>
>>
>> ## Extended description 
>>
>> The json representation for graphics (which I believe is intended for 
>> Jmol), 
>> is **not** correct json. To wit:
>>
>> sage: G = cube((0,0,0),1)
>> sage: obj_list = 
>> sage.plot.plot3d.base.flatten_list(G.flatten().json_repr(G.default_render_params()))
>> sage: j = obj_list[0]
>> sage: print j # json
>>
>> 
>> {vertices:[{x:0.5,y:0.5,z:0.5},{x:-0.5,y:0.5,z:0.5},{x:-0.5,y:-0.5,z:0.5},{x:0.5,y:-0.5,z:0.5},{x:0.5,y:0.5,z:-0.5},{x:-0.5,y:0.5,z:-0.5},{x:0.5,y:-0.5,z:-0.5},{x:-0.5,y:-0.5,z:-0.5}],faces:[[0,1,2,3],[0,4,5,1],[0,3,6,4],[5,4,6,7],[6,3,2,7],[2,1,5,7]],color:'#ff'}
>>
>> sage: import json
>> sage: json.loads(j) # Throws value error
>> 
>> 
>> ## Sage Version: 
>>
>> sage: version()
>> 'SageMath Version 7.1, Release Date: 2016-03-20'
>> 
>> ## Operating System:
>>
>> macOS Sierra Version 10.12.2 
>> 
>> The bug is reproducible in other OSs, though.
>>
>>
>> ## Solution
>>
>> I've traced down the problem, but I don't know if I should submit a pull 
>> request on Github 
>> or what. Advice on what to do is welcome.
>>
>> The problem lies in the file `src/sage/plot/plot3d/index_face_set.pyx`, 
>> where the json is generated. There are two problems with the generated json:
>>
>> 1. The quote signs used in the file are `'` and not `"` as per [Json 
>> spec](
>> http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf
>> ).
>> 2. Every dictionary key is unquoted, which is incorrect. Thus, a point 
>> 
>> {x:0,y:0,z:0}
>> 
>> should really be
>>
>> {"x":0,"y":0,"z":0}
>> 
>> as per Json spec.
>> 
>>
>> In particular, the function `cdef inline format_json_vertex(point_c P):` 
>> should read:
>>
>> cdef inline format_json_vertex(point_c P):
>> cdef char ss[100]
>> cdef Py_ssize_t r = sprintf_3d(ss, '{"x":%g,"y":%g,"z":%g}', P.x, 
>> P.y, P.z)
>> return PyString_FromStringAndSize(ss, r)
>>
>> and lines 871 through 882 should read:
>>
>> if self.global_texture:
>> color_str = '"#{}"'.format(self.texture.hex_rgb())
>> return ['{"vertices":%s,"faces":%s,"color":%s}' %
>> (vertices_str, faces_str, color_str)]
>> else:
>> color_str = "[{}]".format(",".join(['"{}"'.format(
>> Color(self._faces[i].color.r,
>>   self._faces[i].color.g,
>>   self._faces[i].color.b).html_color())
>> for i from 0 <= i < 
>> self.fcount]))
>> return ['{"vertices":%s,"faces":%s,"face_colors":%s}' %
>> (vertices_str, faces_str, color_str)]
>>
>> This will affect doctest output, btw.
>>
>> ## Workaround for existing installations:
>>
>> The silver lining is that even though the Json is incorrect, since it is 
>> very simple, it can be easly corrected inline: 
>>
>> import re
>> def correct_json(s):
>> """
>> Correct the json returned by `Graphics3d.json_repr()`, which 
>> contains the following
>> errors:
>> 1. it uses ' instead of " for strings.
>> 2. dictionary keys are not quoted.
>> 
>> """
>> j = re.sub(r"{\s*(\w+)\s*:\s*", r'{"\1":', s)
>> j = re.sub(r",\s*(\w+)\s*:\s*", r',"\1":', j)
>> j = re.sub(r"'", r'"', j)
>> return j
>>
>> Note that this `correct_json` does **not** work for general json, just 
>> for the particularly simple json produced by the graphics3d module.
>>
>>
>>
>>
>>

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sage-devel+unsubscr...@googlegroups.com.
To post to this group, send email to sage-devel@googlegroups.com.
Visit this group at https://groups.google.com/group/sage-devel.
For more options, visit https://groups.google.com/d/optout.


[sage-devel] Bug report: Incorrect json representation for graphics.

2017-01-25 Thread mjsoto
Markdown below: 

Bug report: Incorrect json representation for graphics.


## Extended description 

The json representation for graphics (which I believe is intended for 
Jmol), 
is **not** correct json. To wit:

sage: G = cube((0,0,0),1)
sage: obj_list = 
sage.plot.plot3d.base.flatten_list(G.flatten().json_repr(G.default_render_params()))
sage: j = obj_list[0]
sage: print j # json


{vertices:[{x:0.5,y:0.5,z:0.5},{x:-0.5,y:0.5,z:0.5},{x:-0.5,y:-0.5,z:0.5},{x:0.5,y:-0.5,z:0.5},{x:0.5,y:0.5,z:-0.5},{x:-0.5,y:0.5,z:-0.5},{x:0.5,y:-0.5,z:-0.5},{x:-0.5,y:-0.5,z:-0.5}],faces:[[0,1,2,3],[0,4,5,1],[0,3,6,4],[5,4,6,7],[6,3,2,7],[2,1,5,7]],color:'#ff'}

sage: import json
sage: json.loads(j) # Throws value error


## Sage Version: 

sage: version()
'SageMath Version 7.1, Release Date: 2016-03-20'

## Operating System:

macOS Sierra Version 10.12.2 

The bug is reproducible in other OSs, though.


## Solution

I've traced down the problem, but I don't know if I should submit a pull 
request on Github 
or what. Advice on what to do is welcome.

The problem lies in the file `src/sage/plot/plot3d/index_face_set.pyx`, 
where the json is generated. There are two problems with the generated json:

1. The quote signs used in the file are `'` and not `"` as per [Json 
spec](http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf).
2. Every dictionary key is unquoted, which is incorrect. Thus, a point 

{x:0,y:0,z:0}

should really be

{"x":0,"y":0,"z":0}

as per Json spec.


In particular, the function `cdef inline format_json_vertex(point_c P):` 
should read:

cdef inline format_json_vertex(point_c P):
cdef char ss[100]
cdef Py_ssize_t r = sprintf_3d(ss, '{"x":%g,"y":%g,"z":%g}', P.x, 
P.y, P.z)
return PyString_FromStringAndSize(ss, r)

and lines 871 through 882 should read:

if self.global_texture:
color_str = '"#{}"'.format(self.texture.hex_rgb())
return ['{"vertices":%s,"faces":%s,"color":%s}' %
(vertices_str, faces_str, color_str)]
else:
color_str = "[{}]".format(",".join(['"{}"'.format(
Color(self._faces[i].color.r,
  self._faces[i].color.g,
  self._faces[i].color.b).html_color())
for i from 0 <= i < 
self.fcount]))
return ['{"vertices":%s,"faces":%s,"face_colors":%s}' %
(vertices_str, faces_str, color_str)]

This will affect doctest output, btw.

## Workaround for existing installations:

The silver lining is that even though the Json is incorrect, since it is 
very simple, it can be easly corrected inline: 

import re
def correct_json(s):
"""
Correct the json returned by `Graphics3d.json_repr()`, which 
contains the following
errors:
1. it uses ' instead of " for strings.
2. dictionary keys are not quoted.

"""
j = re.sub(r"{\s*(\w+)\s*:\s*", r'{"\1":', s)
j = re.sub(r",\s*(\w+)\s*:\s*", r',"\1":', j)
j = re.sub(r"'", r'"', j)
return j

Note that this `correct_json` does **not** work for general json, just for 
the particularly simple json produced by the graphics3d module.




-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sage-devel+unsubscr...@googlegroups.com.
To post to this group, send email to sage-devel@googlegroups.com.
Visit this group at https://groups.google.com/group/sage-devel.
For more options, visit https://groups.google.com/d/optout.