[GSoC] Add reference implementation for Issue Response Time #202
Conversation
This patch adds the python script for the dataframe implementation of issue response time metric. Signed-off-by: Aniruddha Karajgi <akarajgi0@gmail.com>
|
@jgbarah, please have a look at the implementation. I'll add the tests today. Let me know if you think it is ok. |
| # this check removes pull requests from the | ||
| # analysis. | ||
| if 'pull_request' in item['data']: | ||
| return [] |
There was a problem hiding this comment.
Here I am removing pull requests from the issues data (since GitHub treats all pull requests as issues too). Hope this is fine. (this check will of course be in the Issue_Github category class (in _flatten) and is not specific to this metric.)
There was a problem hiding this comment.
As in other cases, i think it is easier for this reference implementation to just assume that the class is fed only with issues. It is easy to do that before instantiating the class. So, please remove that check...
|
|
||
| flat['issue_resp_time'] = np.NaN if len(member_comments) == 0 \ | ||
| else (datetime.now() | ||
| - utils.str_to_date( |
There was a problem hiding this comment.
To find the first comment by a maintainer, I used the following logic. Please have a look and let me know if this is easy to understand / requires other changes. What I am doing here is this:
- Pickout the comments by maintainers and store in
member_comments. - Add the issue response time to
flatby calculated the number of days between today and the first comment by a maintainer (using themin).
There was a problem hiding this comment.
I don't understand this rationale. Time should be from the moment the issue is opened, to the moment it receives the first reponse (not from the first comment to today). If there was still no comment, we should ignore the issue, since it is not correct to say it was responded today...
Are we in sync?
|
|
||
| df = df.resample(period).agg({'issue_resp_time': 'mean'}) | ||
| df['issue_resp_time'] = df['issue_resp_time'].fillna(0) | ||
|
|
There was a problem hiding this comment.
The fillna(0) is required to make it easier to plot the dataframe in notebooks. I could just as well remove this line and in the notebook, just before plotting, I could run this line. What do you prefer?
There was a problem hiding this comment.
I think it is better if you do it when plotting. Otherwise, you're assuming that this is being called before plotting, which maybe is not the case...
This patch adds the reference implementation notebook for the metric Issue Response Time. Signed-off-by: Aniruddha Karajgi <akarajgi0@gmail.com>
|
Before entering into details, I think we should focus on metrics that already done, at least done enough to be relases. Those are the ones listed here. Until we're done with those, let's keep this one in the fridge. I'll try to work on its definition meanwhile. |
|
@Polaris000 : Could you look at this and resolve the questions? |
|
Hi @sgoggins. Thank you for your interest. |
|
@Polaris000, if you don't find the time, don't worry. This metric is not fully defined anyway, so we could just reopen a pr later, when that's done. |
|
Since I see no movement, I´m closing this for now. @Polaris000, please feel free to open a new pr when you have time. |
This pull request adds the reference implementation for Issue Response Time along with tests.