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

(feat) 回归散点图增加显示回归拟合函数相关配置,close #3474 #3476

Merged
merged 12 commits into from
Mar 8, 2023

Conversation

Cuiyansong
Copy link
Contributor

@Cuiyansong Cuiyansong commented Feb 22, 2023

PR includes

Screenshot

Before After
antv-g2plot-regression-old antv-regression-equation-snapshot

@Cuiyansong Cuiyansong changed the title (feat) 回归散点图增加显示回归拟合函数相关配置,#3474 (feat) 回归散点图增加显示回归拟合函数相关配置,close #3474 Feb 23, 2023
@hustcc hustcc requested a review from visiky February 23, 2023 04:06
attrs: {
...defaulTextStyle,
...textStyle,
text: equation,
Copy link
Member

Choose a reason for hiding this comment

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

@visiky 看看默认的公示放到哪里比较合适。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这句话的意思,能详细说一下吗?

Copy link
Member

Choose a reason for hiding this comment

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

equation 文本默认放置的位置在哪里?目前是在 [0, 0] 位置吗?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

目前是基于左上角给了x=20,y=20的偏移量,用户可以自定义x、y值

@visiky
Copy link
Member

visiky commented Feb 23, 2023

这个功能在 G2 5.0 会以组合 mark 的形式实现,在 G2Plot 中把拟合函数作为图表的一个特性挺好的,直接内置到散点图中~

公式展示位置,尽量还是希望可以统一:支持相对定位和绝对定位

@Cuiyansong
Copy link
Contributor Author

Cuiyansong commented Feb 23, 2023

这个功能在 G2 5.0 会以组合 mark 的形式实现,在 G2Plot 中把拟合函数作为图表的一个特性挺好的,直接内置到散点图中~

公式展示位置,尽量还是希望可以统一:支持相对定位和绝对定位

1 - 目前PR的实现方式对G2v5版本是否合适?
2 - 相对定位的方式是值用position的hook去自定义位置吗?参考图表标注功能示例
2.1 - 默认设置,可以改成position: right以及textAlign: end这种模式,其实这就是我想要的。
2.2 - 需要对外暴露postion属性?equationPosition:string | Array<string, string>? 模式是否合适?
3 - 绝对定位默认的位置我原本想参考ECharts将公式展示在右上角(折线终点的位置)文本右对齐,只是不是特别熟悉代码,所以就固定了左上角的位置。“如果存在多种公式辅助线的话,需要额外的标识来使其对应上”这一点目前如何做兼容呢?

@visiky
Copy link
Member

visiky commented Feb 23, 2023

这个功能在 G2 5.0 会以组合 mark 的形式实现,在 G2Plot 中把拟合函数作为图表的一个特性挺好的,直接内置到散点图中~
公式展示位置,尽量还是希望可以统一:支持相对定位和绝对定位

1 - 目前PR的实现方式对G2v5版本是否合适? 2 - 相对定位的方式是值用position的hook去自定义位置吗?参考图表标注功能示例 。 2.1 - 默认设置,可以改成position: right以及textAlign: end这种模式,其实这就是我想要的。 2.2 - 需要对外暴露postion属性?equationPosition:string | Array<string, string>? 模式是否合适? 3 - 绝对定位默认的位置我原本想参考ECharts将公式展示在右上角(折线终点的位置)文本右对齐,只是不是特别熟悉代码,所以就固定了左上角的位置。“如果存在多种公式辅助线的话,需要额外的标识来使其对应上”这一点目前如何做兼容呢?

我晚些再回复哈

@visiky
Copy link
Member

visiky commented Feb 27, 2023

1、相对定位,即标注和折线图的位置是相对的,效果如 Echarts 和 G2 5.0 版本
2、绝对定位,即相对于画布。目前您的实现放置于左上角是没问题的~

@Cuiyansong
Copy link
Contributor Author

👌,对于这个PR还需要那些地方要修改吗?

@Cuiyansong Cuiyansong requested review from hustcc and removed request for visiky February 27, 2023 07:37
@visiky
Copy link
Member

visiky commented Feb 27, 2023

👌,对于这个PR还需要那些地方要修改吗?

非常感谢你的提交~ 完整 PR 我看了,基于现状的 PR 内容,问题不大,可以按照这个形式。下面是一些小的问题,仅供参考:
1、注释英文的前后留空
2、equation 配置,我感觉可以暂时不提供函数回调的方式,等后续丰富之后,回调函数有参数的时候,再行提供

@Cuiyansong
Copy link
Contributor Author

Cuiyansong commented Feb 27, 2023

问题2,已经修改完毕,移除对函数的支持。
问题1,能否在Review代码上标注一下,没有get到哪里需要添加“空格”

@Cuiyansong
Copy link
Contributor Author

Any update?

@hustcc hustcc self-requested a review March 7, 2023 05:28
@visiky visiky merged commit 9f0a25e into antvis:master Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🥰 [FEATURE] 回归散点图增加显示拟合函数的配置项
3 participants