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

自定义模板 #240

Merged
merged 36 commits into from Nov 21, 2017
Merged

自定义模板 #240

merged 36 commits into from Nov 21, 2017

Conversation

kinegratii
Copy link
Collaborator

本文档基于 #228 实现的,新增和优化了若干个功能点。基本上了保持原有的兼容性。

主要内容:

1 开放底层模板引擎类API,支持模板函数

目前可以使用 pyecharts.engine.EchartsEnvironment 引擎类,正如 jinja2 库一样。

from jinja2 import FileSystemLoader
from pyecharts import Bar
from pyecharts.engine import EchartsEnvironment, configure
from pyecharts.utils import write_utf8_html_file

attr = ["衬衫", "羊毛衫", "雪纺衫", "裤子", "高跟鞋", "袜子"]
v1 = [5, 20, 36, 10, 75, 90]
v2 = [10, 25, 8, 60, 20, 80]
bar = Bar("柱状图数据堆叠示例")
bar.add("商家A", attr, v1, is_stack=True)
bar.add("商家B", attr, v2, is_stack=True)

configure(jshost='https://cdn.bootcss.com/echarts/3.7.0')
env = EchartsEnvironment(loader=FileSystemLoader('.'))
tpl = env.get_template('demo.html')
write_utf8_html_file('demo_gen.html', tpl.render(bar=bar))

pyecharts.engine.EchartsEnvironment 类还添加了若干个 echarts_* 模板函数,用于在模板文件生成代码片段。

2 渲染函数增加参数,支持更多自定义参数

目前 render 函数签名如下:

Chart.render(path='render.html', new_version=False, template_name='simple_chart.html', object_name='chart', extra_context=None):

各参数意义如下:

  • path :最终生成文件名称
  • new_version:是否采用新的渲染方式,此为开发过渡参数,将在正式版本后删除
  • template_name: 模板文件名称,其目录可通过 pyecharts.configure() 全局函数进行配置
  • object_name: 模板文件中,该图表类所使用变量的名称
  • extra_context 额外数据字典。

3 优化配置过程

新增统一的配置函数 pyecharts.configure ,在构建渲染图表之前使用该函数可统一配置你所需要的参数,每个参数都有默认值。

原有的 pyecharts.online() 函数将被deprecated,并在之后 remove。

4 优化js依赖文件引入方式

js引入方式共有两种,外部链接方式

<script type="text/javascript" src="https://cdn.bootcss.com/echarts/3.7.0/echarts.min.js"></script>

内部嵌入方式

<script type="text/javascript">
var s = '';
//...
</script>

引入方式的逻辑为:

5 Page 支持列表协议

pyecharts.custom.Page 直接继承 list 类,也能支持长度(len)、索引(index)、切片(splice)、添加(append)、扩展(extend)等操作。

相应的,Page 类移除了 charts 属性,目前可直接迭代 Page 对象本身。

6 其他优化和改变

  • pyecharts.utils.get_resource_dir(*paths) 目前支持不定参数调用
  • 图表类 width/height 支持其他有效css长度形式,如"78px"、"50%" 等,保留原有数字格式,单位为px。

@codecov-io
Copy link

codecov-io commented Oct 31, 2017

Codecov Report

Merging #240 into dev will increase coverage by 0.24%.
The diff coverage is 99.13%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #240      +/-   ##
==========================================
+ Coverage   99.07%   99.32%   +0.24%     
==========================================
  Files          75       80       +5     
  Lines        2603     2812     +209     
==========================================
+ Hits         2579     2793     +214     
+ Misses         24       19       -5
Impacted Files Coverage Δ
test/test_utils.py 100% <100%> (ø) ⬆️
pyecharts/template.py 100% <100%> (ø) ⬆️
test/test_engine.py 100% <100%> (ø)
test/test_custom_render.py 100% <100%> (ø)
pyecharts/base.py 100% <100%> (+6.81%) ⬆️
pyecharts/custom/page.py 100% <100%> (+1.85%) ⬆️
pyecharts/engine.py 100% <100%> (ø)
test/test_template_function.py 100% <100%> (ø)
test/test_page.py 100% <100%> (ø) ⬆️
test/test_conf.py 100% <100%> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4972f96...9f715a6. Read the comment docs.

@@ -71,3 +67,4 @@ def ensure_echarts_is_in_the_front(dependencies):

def online(host=constants.DEFAULT_HOST):
Copy link
Member

Choose a reason for hiding this comment

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

add deprecation note here please. keep the function though. existing user are still using it so let's not break user's code.

@@ -0,0 +1,14 @@
<!DOCTYPE html>
<html lang="zh-CN">
Copy link
Member

Choose a reason for hiding this comment

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

should we aim at international users too?

@chfw
Copy link
Member

chfw commented Nov 3, 2017

I will have a bit more time over the weekend so I will checkout your branch and do manual testing as this is quite a big change. My focus is user defined template path. Stay tuned.

@kinegratii
Copy link
Collaborator Author

kinegratii commented Nov 3, 2017

@chfw Thanks.

@chfw
Copy link
Member

chfw commented Nov 3, 2017

if the custom template lives somewhere else than '.', current python working directory, it will fail to find the custom template.

@@ -78,14 +79,28 @@ def get_js_dependencies(self):
"""
return template.produce_html_script_list(self._js_dependencies)

def render(self, path="render.html"):
def render(self, path='render.html', new_version=False, template_name='simple_chart.html', object_name='chart',
Copy link
Member

Choose a reason for hiding this comment

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

function is too long. better to go through flake8.

@kinegratii
Copy link
Collaborator Author

@chfw Yeah.There is a missing code for this feature,I will add code later.

Make JINJA2 as a lazy object
@chenjiandongx
Copy link
Member

@kinegratii 使用延迟加载的目的是?

default_engine = template.create_buildin_template_engine()
tpl = default_engine.get_template(template_name)
context = {object_name: self}
context.update(extra_context or {})
Copy link
Member

Choose a reason for hiding this comment

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

👍

utils.write_utf8_html_file(path, html)

#
Copy link
Member

Choose a reason for hiding this comment

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

dead code should be removed. git is here to help versioning.

@@ -22,8 +30,8 @@
content = f.read()
CONFIG = json.loads(content)

DEFAULT_JS_LIBRARIES = CONFIG['FILE_MAP']
CITY_NAME_PINYIN_MAP = CONFIG['PINYIN_MAP']
DEFAULT_JS_LIBRARIES = CONFIG['FILE_MAP'] # {<Pinyin>:<Js File Name>}
Copy link
Member

Choose a reason for hiding this comment

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

👍 on the comment

chart_content += chart.render_embed()
chart_content += '<br>'
return chart_content
return '< br > '.join([chart.render_embed() for chart in self])
Copy link
Member

Choose a reason for hiding this comment

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

please get rid of spaces... my fault

<br>


if current_config.js_embed:
contents = Helpers.read_file_contents_from_local(js_names)
return '\n'.join(['<script type="text/javascript">\n{}\n</script>'.format(c) for c in contents])
Copy link
Member

Choose a reason for hiding this comment

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

<script>.... can be put as a constant

@chfw
Copy link
Member

chfw commented Nov 15, 2017

👍 注释适当增加,maxline 设置为 79 吧,有些代码一行太长了。

@chenjiandongx
Copy link
Member

@kinegratii 代码整理后看起来舒服多了


def produce_require_configuration(dependencies, jshost):

def online(host=constants.DEFAULT_HOST):
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need the deprecation warning?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

目前先删除标记吧,个人觉得采用类似 Django 废弃时间线更为合适。对于已有代替的旧有代码分三个移除策略:

  • 不再推荐使用(Not Recommend):仅在更新日志和文档中表明
  • 废弃(Deprecated):使用 warnings 模块表明
  • 移除(Removed):删除相关代码

以上均在次版本号更新(如V0.3.0,V0.4.0等)的变更,直至彻底删除。

DEFAULT_HOST = 'https://chfw.github.io/jupyter-echarts/echarts'
SCRIPT_LOCAL_JSHOST = get_resource_dir('templates', 'js', 'echarts')
Copy link
Member

Choose a reason for hiding this comment

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

SCRIPT_LOCAL_JSHOST should be SCRIPT_IN_THE_PACKAGE, at least it is not hosted :). Once pyecharts is installed, get_resource_dir provides the directory offset '...env/lib/site-packages/pyecharts/'

Copy link
Member

Choose a reason for hiding this comment

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

@kinegratii 这里是有什么问题吗

Copy link
Member

Choose a reason for hiding this comment

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

运行无问题..

Copy link
Member

@chfw chfw left a comment

Choose a reason for hiding this comment

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

The outstanding comments are related to readability only. And this review took such a long time. @kinegratii , thanks for your patience.

@chenjiandongx
Copy link
Member

chenjiandongx commented Nov 17, 2017

@chfw 为了庆祝这个重大的更新,下个版本号为 V0.3.0。也要感谢你的耐心 review

@kinegratii
Copy link
Collaborator Author

本次更新:

1 添加了 Markup 特性,即模板最后生成的字符串再用 Markup 包裹一次。实现了用于 HTML 转义时,设置变量的安全标志。和使用 safe 过滤器的效果是一样的。

Markup 类的主要特点:

  • 直接继承字符类 basestring
  • 增加了 __html__ 方法

2 增加 Flask 整合示例,使得在Flask 项目中也可以使用这些模板函数。

如果没有什么问题,可以先行合并本 PR ,其他文档方面后续再整理。

return self._page_title

@classmethod
def from_charts(cls, *args):
Copy link
Member

Choose a reason for hiding this comment

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

_jsthost cannot be set for now and need to be added for completeness.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

不添加 _jshost 参数,主要基于下面两个原因:

1 如果添加这个参数,会再调用时引起歧义,有以下两种种定义形式:

第一种: Page.from_charts(jshost=None, *args)

这种方式有个问题,就是即使 jshost 无意设置,也需使用 None 占位。

Page.from_charts(chart1, chart2) 调用从字面上是将两个图表合并,实际上只有一个,调用时会把 chart1 传给 jshost

第二种:Page.from_charts(*args, jshost=None)

这个将可选的参数放置在最后,可以解决 Page.from_charts(chart1, chart2) 字面和实际效果一致,但是仅Python3.5+支持

2 从功能上来看,该方法只是 __init__ 方法的补充,不一定非要和其等价。

)


@environmentfunction
Copy link
Member

Choose a reason for hiding this comment

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

👍 well done

Copy link
Member

@chfw chfw left a comment

Choose a reason for hiding this comment

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

please merge it and let @kinegratii fix the issues in another PR. This PR is open too long.

@chfw
Copy link
Member

chfw commented Nov 20, 2017

@kinegratii , please merge the latest code before merging.

@chenjiandongx chenjiandongx merged commit 7620e27 into pyecharts:dev Nov 21, 2017


@environmentfunction
def echarts_js_dependencies_embed(env, *args):
Copy link
Member

Choose a reason for hiding this comment

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

env 变量没有用到是吧

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

模板函数是回调函数,第一个参数必须是 Environment 实例,Jinja2 在调用这个函数时,会把当前引擎实例传入。

@kinegratii kinegratii deleted the feature_template_function branch November 22, 2017 01:29
chenjiandongx added a commit that referenced this pull request Mar 26, 2019
chenjiandongx added a commit that referenced this pull request Mar 30, 2019
chenjiandongx added a commit that referenced this pull request Mar 30, 2019
chenjiandongx added a commit that referenced this pull request Mar 30, 2019
chenjiandongx added a commit that referenced this pull request Mar 30, 2019
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.

None yet

4 participants