mistercrunch commented on issue #3518: Full Annotation Framework
URL: 
https://github.com/apache/incubator-superset/pull/3518#issuecomment-352194429
 
 
   I pulled the branch over and this is looking really solid at this point. I 
think we should get this in as soon as a few things are ironed out.
   
   A few things:
   
   * looked like the "range filter" option is not compatible with the "Superset 
Annotation" type, meaning as the x axis range change the annotation doesn't 
move along with it. This is a bug prior to this PR too, maybe the solution is 
to disable annotations when Range Filter is selected, even to raise and say 
"range filter isn't compatible with annotations at the moment" when users 
combine both
   
   * i set up a wide "Superset annotation" and it doesn't show the rectangle 
(only the left bound), which makes me think we should add some annotations in 
`load_examples` for demo and user testing purposes (not sure if it has to be 
in-scope for this PR though)
   
   * I looked into `mathjs` before and other alternatives like `safe-eval`, 
`evel` and others. Eventually I'd like to add a new permission called `coder` 
or something and allow people that have that to write JS functions as part of 
chart definitions. We should have only one of these libs, not sure which one is 
best. If math.js is a subset of `safe-eval` we can probably refactor later

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to