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

Implementing the current Report using new functions #66

Closed
aswanipranjal opened this issue Jun 6, 2018 · 4 comments
Closed

Implementing the current Report using new functions #66

aswanipranjal opened this issue Jun 6, 2018 · 4 comments

Comments

@aswanipranjal
Copy link
Contributor

aswanipranjal commented Jun 6, 2018

I've been working on redesigning the functions and classes so as to calculate the Metrics in a better way.

  • This issue is about trying to implement the current reports that Manuscripts produces using these new functions. I am limiting this issue to only calculate the Metrics defined under Github Issues, PRs and Git Commits, as this is for testing purposes and most of the Metrics are under these 3 categories.

  • I was unable to add these new functions directly into manuscripts(by creating a branch and generating a report using these functions like what happens currently) because the current functions use Inherited classes for each metric and are different than what I have implemented.

  • These new functions cannot just be plugged into manuscripts and will mostly require a complete makeover of the manuscripts project, if it is decided that they are correct and should be implemented.

  • The new functions and classes have been created, keeping in mind the metrics under GMD and others so that calculating them becomes easier and useful for the users.

I've implemented the metrics in this notebook and this is the reference pdf showing the metrics calculated by manuscripts using the old functions.

@jgbarah @valeriocos @acs please have a look at the notebook!

@jgbarah
Copy link
Contributor

jgbarah commented Jun 6, 2018

I like the approach. I would think a bit more about the Metric class and its subclasses. For example, instead of

closed_pr = Metric("http://localhost:9200/", github_index, start_date, end_date)
closed_pr.add_query({"pull_request":"true"})

I would define a class "Pull_Request", which would inhering from "Metric" (or maybe from "GitHub"), internally adding the query {"pull_request":"true"}, because if you want pull requests, you always want that filter.

In addition, some fields that are now in the instantiation of the metric,such as start_date and end_date, I would implement them also as methods of the metric, so that you can just chain them if needed. So the above code would be something like:

closed_pr_in_period = Pull_Requests(github_index) \
    .since(start_date) \
    .until(end_date)

github_index would be an object with all the data needed to start the query, that is, in our case, an elasticsearch connection, and an index name. The complete code would be then:

github_index = Index(con=elasticsearch_con, index='github')
closed_pr_in_period = Pull_Requests(github_index) \
    .since(start_date) \
    .until(end_date)

Note that since, until, etc. would be implemented as functions of Pull_Requests, returning Pull_Requests objects.

What do you think?

@jgbarah
Copy link
Contributor

jgbarah commented Jun 6, 2018

WRT to how to implement this on the current Manuscripts, I see the problem you mention of this being completely different from the current implementation. Maybe we can start with a new directory in it (manuscripts2, for example), and include in it all the stuff, until when we can compute at least the current reports...

@aswanipranjal
Copy link
Contributor Author

aswanipranjal commented Jun 7, 2018

The idea of having a PullRequests and Issues class sounds good. But I fear that we might get into the same design pattern as the previous ones. Although, we can differentiate by adding some functions specific to the class, in the future.
I'll implement the PullRequests and Issues classes.

In addition, some fields that are now in the instantiation of the metric,such as start_date and end_date, I would implement them also as methods of the metric, so that you can just chain them if needed.

+1. I'll replace set_range function with these. The since and until functions will also need a field too to set range to though.

Maybe we can start with a new directory in it (manuscripts2, for example), and include in it all the stuff, until when we can compute at least the current reports...

@jgbarah I was able to compute the metrics from Git and Github. Should I start looking at other data sources?

@jgbarah
Copy link
Contributor

jgbarah commented Jul 6, 2018

Should we close this issue?

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