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

Customized KPI Calculation #83

Closed
4 tasks done
dhblum opened this issue Jul 1, 2019 · 7 comments
Closed
4 tasks done

Customized KPI Calculation #83

dhblum opened this issue Jul 1, 2019 · 7 comments

Comments

@dhblum
Copy link
Collaborator

dhblum commented Jul 1, 2019

This issue is to integrate the customized KPI calculation.

To Do:

  • Merge from Sen's development
  • More documentation in docstrings and readme.
  • Delineate required and optional keys in kpi config.
  • Make data_point_num not required for be specified for all kpis.
@SenHuang19
Copy link
Contributor

I accidentally commit to the issue83_customKPI branch directly

@dhblum
Copy link
Collaborator Author

dhblum commented Jul 15, 2019

No problem, this works too!

@dhblum
Copy link
Collaborator Author

dhblum commented Jul 15, 2019

I've merged the most recent master to the branch.

@dhblum
Copy link
Collaborator Author

dhblum commented Jul 16, 2019

Hey @SenHuang19 , here are comments I have:

  • More documentation could be added, particularly as docstrings for method and class descriptions and a guide (in the form of a readme) for potential users who want to adopt the capability as you demonstrated.
  • It seems that the kpi configuration file has a number of keys that are required, and some that are not. I suggest somehow delineating between which are required and which are not.
  • I don't think data_point_num needs to be in all KPI configuration files, but instead could be handled within the KPI calculation classes themselves, since they are classes and have memory. Removing this from the config or making it optional simplifies the interface I think.

I've added a suggested to-do at the top comment.

@SenHuang19
Copy link
Contributor

Update the pull request #95
Specifically, I made the customized KPI compatible with both testcase 1 and 2

@dhblum
Copy link
Collaborator Author

dhblum commented Jan 10, 2020

I merged #95 and opened #143 for merge to master.

@dhblum dhblum closed this as completed in 2f7736a Feb 17, 2020
@dhblum
Copy link
Collaborator Author

dhblum commented Feb 17, 2020

Closed by #143.

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

No branches or pull requests

2 participants