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
Simplify and remove javascript from html_annotate templates. #2793
Conversation
Thanks for starting work on this feature, seems like you've got some pretty useful and usable results already. I noted that this was py3.5 only and that:
(Please could you elaborate on this ^). The annotations also work via setting env flags and a subprocess, I'm not familiar with the internals of magics, is this an issue, what would really improve it? Thanks again. |
Well right now I need to call What I'd like is
Or even
So that I can pass that to my own template ... or do whatever. As this might not be easy,
Which gives me a json file (or send it to stdout/err), so that I can load it and run it through my own template. Probably just
From a high level point of view, magics are functions that takes a string (the code typed by the user) and return a python object. The rest is up to the implementor. They have access to internal kernel state if need be. Usually I try to design things to not access the filesytem. What would it take to make numba --annotate-html a flask-powered webserver would be a good mental model. |
Soem extra tweaks: In particular we avoid to color tokens, but color background in The style is a compromize betwen loking good as a standalone html file Standalone file: In both the blue-ish row is the one hovered by my cursor. |
Thanks for those extra tweaks, this is looking really good. Highlighting the background instead to allow syntax highlighting is a great idea. RE #2793 (comment) I think this is something we could probably make work given there is a dictionary and it is just given to a template. Here is a quick hack as an intermediary that might help in terms of working out what could be done templating/rendering wise, this isn't something to use long term as it a) still writes to file, b) has manual flag setting, but it gets out an ordered dict of the annotation info that is passed to jinja2 import numpy as np
from tempfile import TemporaryFile
from numba import jit
from numba.compiler import Flags
class clazz(object):
def __init__(self, arr):
self._arr = arr
@property
def arr(self):
return self._arr
@arr.setter
def arr(self, value):
self._arr = value
def foo(a, c):
c.arr += 12
for i in range(len(a)):
a[i] = np.sqrt(i) + 7
if i % 8 + 1 > 4:
a[i] -= np.pi
return a
A = np.arange(100.)
c1 = clazz(11)
c2 = clazz(11)
print(foo(A, c1), c1.arr)
print(jit(foo)(A, c2), c2.arr)
JIT_FLAGS = Flags()
JIT_FLAGS.set("nrt")
JIT_FLAGS.set("enable_pyobject")
JIT_FLAGS.set("enable_pyobject_looplift")
def compile_annotate(func, *args, flags=JIT_FLAGS):
from numba import sigutils
from numba.compiler import compile_isolated
from numba.dispatcher import OmittedArg
from numba.typing.typeof import Purpose
from numba.annotations.type_annotations import TypeAnnotation
from numba import typeof, types
def typeof_pyval(val):
try:
tp = typeof(val, Purpose.argument)
except ValueError:
tp = types.pyobject
else:
if tp is None:
tp = types.pyobject
return tp
argtypes = []
for a in args:
if isinstance(a, OmittedArg):
argtypes.append(types.Omitted(a.value))
else:
argtypes.append(typeof_pyval(a))
cres = compile_isolated(func, tuple(argtypes), flags=flags)
# force annotation generation, have to write to file at present
with TemporaryFile('wt') as f:
cres.type_annotation.html_annotate(f)
return TypeAnnotation.func_data
annotations = compile_annotate(foo, A, c2) Numba core dev meetings are on Thursdays, I'll raise splitting the annotation code into a generation and processing phase, I think it makes sense to do this anyway. Thanks again. |
BTW it seem that with above snippet numba is not able to optimize the functions and sees pyobjects everywhere, even if it infers the signature correctly. |
Thanks for the extra updates (#2793 (comment)), that's really great :) I'm glad the hack allowed you to progress. RE: #2793 (comment), yes, please if there's some data structure that's going to be easier for these external tools to consume then it'd be good to hear about it. I expect we can just write an adaptor as needed. RE: #2793 (comment) Seems like here #2793 (comment) the looplifting was highlighted green, so not sure what has gone on to make it not? |
Ok, so far in
For example. That assumes HTML, where I can see where we could have an ANSI term rendering. |
Ah yes, thanks, I see what you mean, I'll have a think about how to restructure this so it is more consumable by other things. Do you have an example/screenshot of ANSI term rendering of something similar to this I could have a look at please? |
Well I can even make that in a couple of lines with Jinja: I cleaned annotation with: from collections import OrderedDict
def count_nbsp(strn):
if isinstance(strn, list):
return [count_nbsp(x) for x in strn]
count = len(strn)//6
assert strn == ' ' * count, strn
return count
lc = OrderedDict()
for k, v in annotations.items():
l = {}
for kk, vv in v.items():
if kk.endswith('_indent'):
tmp = {x:count_nbsp(y) for x,y in vv.items()}
l[kk] = tmp
else:
l[kk] = vv
lc[k] = l (really ugly but does the job done) and use pygments TermFormatter to get ansi ansi-highlighted code. |
That looks good :) I've added this stuartarchibald@c764e86 which may help. It splits the HTML and general creation of an annotation dictionary apart (no more writes files needed). It also removes markup specific information from the dictionary. Finally it exposes this information as the method Quick demo: import numpy as np
from numba import jit
class clazz(object):
def __init__(self, arr):
self._arr = arr
@property
def arr(self):
return self._arr
@arr.setter
def arr(self, value):
self._arr = value
def foo(a, c):
c.arr += 12
for i in range(len(a)):
a[i] = np.sqrt(i) + 7
if i % 8 + 1 > 4:
a[i] -= np.pi
return a
A = np.arange(100.)
c1 = clazz(11)
c2 = clazz(11)
jitfoo = jit(foo)
jitfoo(A, c2)
from pprint import pprint
pprint(jitfoo.get_annotation_info(jitfoo.signatures[0])) gives: OrderedDict([(('numba_annotate.py:19',
'(array(float64, 1d, C), pyobject) -> pyobject'),
{'filename': 'numba_annotate.py',
'funcname': 'foo',
'ir_indent': {18: 8,
19: 0,
20: 14,
21: 0,
22: 14,
23: 0,
24: 0,
25: 2,
26: 0,
27: 0,
28: 4},
'ir_lines': {18: [('label 0', ''),
('del $const0.4', ''),
('del $0.3', ''),
('del c', ''),
('del $0.5', '')],
19: [],
20: [('a = arg(0, name=a) :: ', 'pyobject'),
('c = arg(1, name=c) :: ', 'pyobject'),
('$0.3 = getattr(value=c, attr=arr) :: ',
'pyobject'),
('$const0.4 = const(int, 12) :: ',
'pyobject'),
('$0.5 = inplace_binop(fn=+=, immutable_fn=+, '
'lhs=$0.3, rhs=$const0.4, static_lhs=<object '
'object at 0x7fe2610556d0>, '
'static_rhs=<object object at '
'0x7fe2610556d0>) :: ',
'pyobject'),
('(c).arr = $0.5', ''),
('jump 14', ''),
('label 14', '')],
21: [],
22: [('jump 16', ''),
('label 16', ''),
('$47 = const(LiftedLoop, '
'LiftedLoop(<function foo at '
'0x7fe260ffaea0>)) :: XXX Lifted Loop XXX',
''),
('$48 = call $47(a, func=$47, args=[Var(a, '
'numba_annotate.py (20))], kws=(), '
'vararg=None) :: XXX Lifted Loop XXX',
''),
('del $47', ''),
('a = static_getitem(value=$48, index=0, '
'index_var=None) :: ',
'pyobject'),
('del $48', ''),
('jump 88', '')],
23: [],
24: [],
25: [('label 88', ''), ('del a', '')],
26: [],
27: [],
28: [('$88.2 = cast(value=a) :: ', 'pyobject'),
('return $88.2', '')]},
'num_lifted_loops': 1,
'python_indent': {18: 0,
19: 0,
20: 4,
21: 0,
22: 4,
23: 8,
24: 8,
25: 12,
26: 0,
27: 0,
28: 4},
'python_lines': [(18, 'def foo(a, c):'),
(19, ''),
(20, 'c.arr += 12'),
(21, ''),
(22, 'for i in range(len(a)):'),
(23, 'a[i] = np.sqrt(i) + 7'),
(24, 'if i % 8 + 1 > 4:'),
(25, 'a[i] -= np.pi'),
(26, ''),
(27, ''),
(28, 'return a')],
'python_tags': {18: '',
19: '',
20: 'object_tag',
21: '',
22: 'lifted_tag',
23: '',
24: '',
25: '',
26: '',
27: '',
28: 'object_tag'}})]) |
ir_indent seem to be wrong in your patch:
Ir indent is a list of indent level for each line of the ir_code, you seem to do the sum of indent for all the line. |
See Carreau@d14e794 as a fix. |
For I'm wondering if this can be used to guess whether IR should be shown by default or not. I'm guessing when you look at IR, you are interested in seeing where PyObject are allocated. In the HTML repr that could be used to select wether things are collapse or not. On ANSI representation i'm thinking of not showing the IR for |
I pushed a working prototype on top of @stuartarchibald patch. There are a few questions: When in a notebook and you redefine a function the old annotation stay attached to the new version – which can be confusing. We need to figure out an API that make seeing things for a given signature, I guess. You don't want to dump the annotated version for all the available signatures. Not sure is |
@Carreau sorry for the bit of a delay! Thanks for adding the prototype, it's looking really good, the syntax highlighting and code folding is really useful. I agree with your assessment over As to #2793 (comment),
Thanks, noted, this is probably a caching bug.
Agreed. I've had a small hack at it. How does something like this seem workflow/API wise? (basically, In [1]: import numba
In [2]: from numba_annotate import Annotate
In [3]: @numba.jit
...: def test(q):
...: res = 0
...: for i in range(q):
...: res += i
...: return res
...:
...:
In [4]: test(10)
Out[4]: 45
In [5]: annotator = Annotate(test)
In [6]: annotator.show_signatures()
0: (int64,)
In [7]: annotator.show_annotated(0)
Out[7]:
Function name: test
In file: <ipython-input-3-86458ba63b34>
With signature: (int64,) -> int64
1: @numba.jit
-- label 0
-- del $const0.1
2: def test(q):
3: res = 0
-- q = arg(0, name=q) :: int64
-- $const0.1 = const(int, 0) :: int64
-- res = $const0.1 :: int64
-- jump 4
-- label 4
4: for i in range(q):
-- jump 6
-- label 6
-- $6.1 = global(range: <class 'range'>) :: Function(<class 'range'>)
-- $6.3 = call $6.1(q, func=$6.1, args=[Var(q, <ipython-input-3-86458ba63b34> (3))], kws=(), vararg=None) :: (int64,) -> range_state_int64
-- del q
-- del $6.1
-- $6.4 = getiter(value=$6.3) :: range_iter_int64
-- del $6.3
-- $phi14.1 = $6.4 :: range_iter_int64
-- del $6.4
-- jump 14
-- label 14
-- $14.2 = iternext(value=$phi14.1) :: pair<int64, bool>
-- $14.3 = pair_first(value=$14.2) :: int64
-- $14.4 = pair_second(value=$14.2) :: bool
-- del $14.2
-- $phi16.1 = $14.3 :: int64
-- $phi28.1 = $14.3 :: int64
-- del $phi28.1
-- del $14.3
-- $phi28.2 = $phi14.1 :: range_iter_int64
-- del $phi28.2
-- branch $14.4, 16, 28
-- label 16
-- del $14.4
-- i = $phi16.1 :: int64
-- del $phi16.1
-- del i
-- del $16.4
5: res += i
-- $16.4 = inplace_binop(fn=+=, immutable_fn=+, lhs=res, rhs=i, static_lhs=<object object at 0x7f9279d96d10>, static_rhs=<object object at 0x7f9279d96d10>) :: int64
-- res = $16.4 :: int64
-- jump 14
-- label 28
-- del $phi16.1
-- del $phi14.1
-- del $14.4
-- jump 30
-- label 30
-- del res
6: return res
-- $30.2 = cast(value=res) :: int64
-- return $30.2
In [8]: test(11.7)
Out[8]: 55
In [9]: annotator.show_signatures()
0: (int64,)
1: (float64,)
In [10]: annotator.show_annotated(1)
Out[10]:
Function name: test
In file: <ipython-input-3-86458ba63b34>
With signature: (float64,) -> int64
1: @numba.jit
-- label 0
-- del $const0.1
2: def test(q):
3: res = 0
-- q = arg(0, name=q) :: float64
-- $const0.1 = const(int, 0) :: int64
-- res = $const0.1 :: int64
-- jump 4
-- label 4
4: for i in range(q):
-- jump 6
-- label 6
-- $6.1 = global(range: <class 'range'>) :: Function(<class 'range'>)
-- $6.3 = call $6.1(q, func=$6.1, args=[Var(q, <ipython-input-3-86458ba63b34> (3))], kws=(), vararg=None) :: (int32,) -> range_state_int32
-- del q
-- del $6.1
-- $6.4 = getiter(value=$6.3) :: range_iter_int32
-- del $6.3
-- $phi14.1 = $6.4 :: range_iter_int32
-- del $6.4
-- jump 14
-- label 14
-- $14.2 = iternext(value=$phi14.1) :: pair<int32, bool>
-- $14.3 = pair_first(value=$14.2) :: int32
-- $14.4 = pair_second(value=$14.2) :: bool
-- del $14.2
-- $phi16.1 = $14.3 :: int32
-- $phi28.1 = $14.3 :: int32
-- del $phi28.1
-- del $14.3
-- $phi28.2 = $phi14.1 :: range_iter_int32
-- del $phi28.2
-- branch $14.4, 16, 28
-- label 16
-- del $14.4
-- i = $phi16.1 :: int32
-- del $phi16.1
-- del i
-- del $16.4
5: res += i
-- $16.4 = inplace_binop(fn=+=, immutable_fn=+, lhs=res, rhs=i, static_lhs=<object object at 0x7f9279d96d10>, static_rhs=<object object at 0x7f9279d96d10>) :: int64
-- res = $16.4 :: int64
-- jump 14
-- label 28
-- del $phi16.1
-- del $phi14.1
-- del $14.4
-- jump 30
-- label 30
-- del res
6: return res
-- $30.2 = cast(value=res) :: int64
-- return $30.2
|
Thanks, No problem I'm in no hurry to merge that.
and
I wrote the html export for Cython Annotate as well long time ago, maybe at some point in the future that would be helpful to try to make the display code reusable. |
This came up today in the scikit-image/dask sprint. I'm curious, what is the status of this PR? Are there current issues holding it up from being merged? |
Having some people actually test it a bit to report workflow issues/improvement, and maybe stabilize a tiny bit the intermediate json representation. I guess this could be marked as unstable, and refine across releases. Probably need also a couple of css fixes across browsers. Feel free to take over, I can give anyone push access to my fork. |
numba/tests/test_annotations.py
Outdated
@@ -83,7 +83,7 @@ def udt(x): | |||
|
|||
# Regex pattern to check for the "lifted_tag" in the line of the loop | |||
re_lifted_tag = re.compile( | |||
r'<td class="lifted_tag">\s*' | |||
r'<class class="lifted_tag">\s*' | |||
r'[ ]+for i in range\(x\): # this line is tagged\s*' | |||
r'</td>', re.MULTILINE) |
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.
mistake here, leave td most likely. Or was it meant to be span or div ?
numba_annotate.py
Outdated
@@ -0,0 +1,245 @@ | |||
""" |
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.
Need to figure out where this file should go.
This simplify and remove javascript from the annotate templates html, and allow to expand individual lines instead of full functions. This also only use details and summary elements, in order to make this later compatible with html clients like JupyterLab and Nteract that do not have global javascript context in which you can inject Javascript without writing full extensions. In particular we avoid to color tokens, but color background in gree-ish/red-ish, this should allow to embed syntax highlighted code and have proper coloring of tokens using pygments. The style is a compromize betwen loking good as a standalone html file and looking good when used in jupyterLab with the numba annotate magic.
This should likely be folded into |
:-( now I hit object that |
rebased on |
Codecov Report
@@ Coverage Diff @@
## master #2793 +/- ##
==========================================
- Coverage 85.75% 85.67% -0.09%
==========================================
Files 327 328 +1
Lines 68654 68979 +325
Branches 7769 7830 +61
==========================================
+ Hits 58874 59095 +221
- Misses 8538 8632 +94
- Partials 1242 1252 +10 |
The |
@Carreau Thanks for the fixups, I think this is almost ready. We can add additional features like the signature parse/lookup as per #2793 (comment) later. Things to do now include:
I'm happy to do any of this as you see fit, please just shout. Thanks again. |
@Carreau I've written stuartarchibald@8f6e4b6 to address most of the above, please feel free to merge in as you see fit. I'm inclined to not worry about the style kwarg for now. WRT the rendering, depending on how easy it is to fix it might be something to push back until some feedback is obtained. @sklam please could you code review? I'm happy with it but have also put code in! Thanks all. |
numba/dispatcher.py
Outdated
print or return annotated source with Numba intermediate IR | ||
|
||
Pass `pretty=True` to attempt color highlighting, and HTML rendering in | ||
Jupyter and IPython by returning an Annotate Object. Pygments |
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.
"Pygments"? sentence incomplete
numba/pretty_annotate.py
Outdated
|
||
pylex = PythonLexer() | ||
|
||
def hllines(code:str, style): |
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.
Type annotation is not supported in py2.
self.annotate_raw() | ||
# make a deep copy ahead of the pending mutations | ||
func_data = copy.deepcopy(self.func_data) | ||
#def add_in_nbsp(key): |
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.
remove this line?
numba/dispatcher.py
Outdated
print('=' * 80, file=file) | ||
else: | ||
from .pretty_annotate import Annotate | ||
return Annotate(self, style=style) |
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.
Need to add test to exercise this branch
Adds documentation, bans pretty+file in inspect_types and tests.
As title.
@Carreau As per the suggested #2793 (comment), I've pushed up on your active branch, thanks. @sklam thanks for review, the last two commits should address the outstanding issues. I think this can make it into 0.39. Huge thanks for the effort @Carreau :) |
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.
Approve for merging. Pending build farm status.
Thanks for the review and merge approval @sklam, buildfarm should be good to run this now. Merging. |
This simplify and remove javascript from the annotate templates html,
and allow to expand individual lines instead of full functions.
This also only use details and summary elements, in order to make this
later compatible with html clients like JupyterLab and Nteract that do
not have global javascript context in which you can inject Javascript
without writing full extensions.
See some of #2788 comments , cc @stuartarchibald
There is a tiny bit of code repetition, but not sure it's worth writing a jinja macro just for that.
New vs Old.