search

src-d / ml-core

source{d} MLonCode foundation - core algorithms and models.
Other
14 stars 16 forks source link

Add Bblfsh3 compatibility #30

Closed Guillemdb closed 5 years ago

vmarkovtsev commented 5 years ago

@Guillemdb Please rebase, I see merge commits. Also, the CI does not pass.

Guillemdb commented 5 years ago

@zurk Can you please confirm that the output of the failed tests is corrects and that the differences are due to the new bblfsh version? when you do, I will add the new output as the test values.

EDIT:

zurk commented 5 years ago

Well, I cannot tell. Just try to set old version and see if these tests pass. You should take a look at how identifiers are extracted. Maybe the names are different or something like that.

No idea about modelforge error. Looks like it is related to nn_splitter.

Guillemdb commented 5 years ago

I am afraid is not that simple, because setting the old version involves changing 20 files. The names are the same (changing string capitalization) but there are minor dfiferences. (the diferences are printed in the travis build, you can check them here.

I don't know how this is supposed to work in the first place, but to me the new outputs make sense.

Guillemdb commented 5 years ago

Update: I looked again at the tests, and I was wrong. They do not make sense at all. I am not extracting the lines and the tokens correctly, but I am already working on it. Once I am sure I know how to replicate correctly the old API I will make the PR to bblfsh and an update on the community channel.

Regarding the role identifiers. Should I make the conversion back to integers, or should I leave them as strings?

zurk commented 5 years ago

As was discussed I am taking over this PR to let @Guillemdb work on the more important stuff

zurk commented 5 years ago

Work continues in https://github.com/src-d/ml-core/pull/31