-
Notifications
You must be signed in to change notification settings - Fork 541
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
Feature metric jsd (#66) #71
Conversation
* [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Apply suggestions from code review Co-authored-by: Zhongsheng Ji <9573586@qq.com> * jsd * jsd * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * base更新 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Delete .idea directory --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: MoooCat <141886018+MooooCat@users.noreply.github.com> Co-authored-by: Zhongsheng Ji <9573586@qq.com>
Apply Sweep Rules to your PR?
|
sdgx/metrics/column/jsd.py
Outdated
self.metric_name = "jensen_shannon_divergence" | ||
|
||
@classmethod | ||
def calculate(cls, real_data, synthetic_data, discrete, cols): |
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.
Suggest add some type hint here
Co-authored-by: Zhongsheng Ji <9573586@qq.com>
Thanks to @Wh1isper for the carefully review. I will add some descriptive comments for input parameters in the method |
Actually type hint and docstring are all we need Like we did in https://github.com/hitsz-ids/synthetic-data-generator/blob/main/sdgx/data_processors/base.py Example: # type hint example
# For python 3.8 and 3.9
from __future__ import annotations
@classmethod
def calculate(cls, real_data: pd.DataFrame, synthetic_data: pd.DataFrame, discrete: list[str] | None, cols: list[str] | None) -> pd.DataFrame:
"""
What is this function
Args:
real_data (pd.DataFrame): balabala...
....
""" |
I'd be inclined to add this to all public methods, especially those that require subclass implementation. This is very intuitive for developers and users, and allows static checking tools to work well. |
Co-authored-by: Zhongsheng Ji <9573586@qq.com>
for more information, see https://pre-commit.ci
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.
LGTM. Thanks!
* Update metric base-class * Update column metric base class * Update Metric Base Class * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Apply suggestions from code review Co-authored-by: Zhongsheng Ji <9573586@qq.com> * Feature: Add metric jsd (#66) (#71) * Feature metric jsd (#66) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Apply suggestions from code review Co-authored-by: Zhongsheng Ji <9573586@qq.com> * jsd * jsd * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * base更新 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Delete .idea directory --------- Co-authored-by: Jinhang Su <171846802@qq.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: MoooCat <141886018+MooooCat@users.noreply.github.com> Co-authored-by: Zhongsheng Ji <9573586@qq.com> * Apply suggestions from code review Co-authored-by: Zhongsheng Ji <9573586@qq.com> * Update type hint and comments. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Apply suggestions from code review Co-authored-by: Zhongsheng Ji <9573586@qq.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Jinhang Su <171846802@qq.com> Co-authored-by: sjh120 <171846802@qq.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Zhongsheng Ji <9573586@qq.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix typo error unnecessary imports are also removed. * Add type hints and comments * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * update input check methods The format of some codes has also been adjusted. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Zhongsheng Ji <9573586@qq.com> Co-authored-by: Jinhang Su <171846802@qq.com>
* Update metric base-class * Update column metric base class * Update Metric Base Class * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Apply suggestions from code review Co-authored-by: Zhongsheng Ji <9573586@qq.com> * Feature: Add metric jsd (#66) (#71) * Feature metric jsd (#66) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Apply suggestions from code review Co-authored-by: Zhongsheng Ji <9573586@qq.com> * jsd * jsd * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * base更新 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Delete .idea directory --------- Co-authored-by: Jinhang Su <171846802@qq.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: MoooCat <141886018+MooooCat@users.noreply.github.com> Co-authored-by: Zhongsheng Ji <9573586@qq.com> * Apply suggestions from code review Co-authored-by: Zhongsheng Ji <9573586@qq.com> * Update type hint and comments. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Apply suggestions from code review Co-authored-by: Zhongsheng Ji <9573586@qq.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Jinhang Su <171846802@qq.com> Co-authored-by: sjh120 <171846802@qq.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Zhongsheng Ji <9573586@qq.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix typo error unnecessary imports are also removed. * Add type hints and comments * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * update input check methods The format of some codes has also been adjusted. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Zhongsheng Ji <9573586@qq.com> Co-authored-by: Jinhang Su <171846802@qq.com>
for more information, see https://pre-commit.ci
Apply suggestions from code review
jsd
jsd
[pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
base更新
[pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
Description
The JSD single column metric.
Motivation and Context
Since the base class of metric(Branch: feature-metric-base) has not yet been merged into the main branch, this JSD branch will first merge into the feature metric base and then merge into the main branch.
Due to changes in some components, it is necessary to make some modifications to the examples before merging.
Types of changes
Checklist: