Skip to content

Conversation

dpordomingo
Copy link
Contributor

@dpordomingo dpordomingo commented Mar 28, 2018

required by #198 and https://github.com/src-d/backlog/issues/1202
fixes #229

As defined by #198 (comment), the ML Feature Extractor API will take a couple of UAST, and it will return a bunch of features per each UAST, and a score as a pair-feature

This PR prepares the frontend to do one call per FilePair being reviewed to obtain the features of each file, and the score itself.
This PR prepares the backend to answer following the expected schema.

No UI changes were done.

It was also added a
3445eb0 commit to fix #229 because otherwise the FilePairs beyond the first one cannot be reviewed.

Pending work

To be done once the ML Feature Extractor API is ready, but that does not block this PR:

@dpordomingo dpordomingo requested review from carlosms and smacker March 28, 2018 14:32
@dpordomingo dpordomingo self-assigned this Mar 28, 2018
@dpordomingo dpordomingo added bug Something isn't working enhancement Improvements are needed over an existent feature feature-request New feature or request form the users labels Mar 28, 2018
@dpordomingo dpordomingo force-pushed the feature-extractor-api branch from 2e4b936 to 3445eb0 Compare March 28, 2018 14:50
@dpordomingo dpordomingo changed the title Feature extractor api Request features from Review section with one call Mar 28, 2018
Copy link
Contributor

@carlosms carlosms left a comment

Choose a reason for hiding this comment

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

👍

@smacker
Copy link
Contributor

smacker commented Apr 3, 2018

I didn't get why we need to change it to 1 call now.
As I understand there is no API yet. And we can actually use https://github.com/src-d/gemini/blob/master/src/main/python/server.py as such api. It accepts 1 uast + params for extraction and returns list of features.

I would better extract fix for #229 in a separate commit and wait with other changes as api isn't ready yet.

on another hand changes are small, so if you want to merge it, it's okay.

@dpordomingo
Copy link
Contributor Author

dpordomingo commented Apr 3, 2018

@smacker all your doubts are properly explained in the PR description; for example:

  • we need to call the API with two UAST bec it will return the similarity score between them,

[...] the ML Feat. Extr. API will take a couple of UAST,and it will return [...] a score as a pair-feature

@dpordomingo dpordomingo force-pushed the feature-extractor-api branch from 3445eb0 to 87b5b79 Compare April 3, 2018 15:54
@dpordomingo dpordomingo mentioned this pull request Apr 3, 2018
Copy link
Contributor

@bzz bzz left a comment

Choose a reason for hiding this comment

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

Although #233 (comment) has the point, the explanation about necessity of this work holds and even if it's a bit preliminary, it LGTM.

@dpordomingo dpordomingo removed the request for review from smacker April 6, 2018 12:59
@dpordomingo dpordomingo merged commit af491fb into src-d:master Apr 6, 2018
@dpordomingo dpordomingo deleted the feature-extractor-api branch April 6, 2018 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement Improvements are needed over an existent feature feature-request New feature or request form the users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reviewer panel is not fetching features when reviewed FilePair changes

4 participants