Skip to content
This repository has been archived by the owner. It is now read-only.

Refactor vandalism detection package #12

Merged
merged 29 commits into from Jan 23, 2019

Conversation

@GezimSejdiu
Copy link
Member

@GezimSejdiu GezimSejdiu commented Jan 8, 2019

This PR does a cleanup and refactor the vandalism detection package. It introduces a common layer for feature extractions since it is done on the string manipulation only and can be used further by Spark and Flink module.

Best regards,

@GezimSejdiu GezimSejdiu requested review from dgraux and HajiraJabeen Jan 8, 2019
@dgraux
Copy link
Member

@dgraux dgraux commented Jan 8, 2019

Hello @GezimSejdiu
Thank you very much. Are you also considering adding a test SANSA-ML/sansa-ml-spark/src/test/scala/net/sansa_stack/ml/spark/outliers/?

Loading

dgraux
dgraux approved these changes Jan 8, 2019
Copy link
Member

@dgraux dgraux left a comment

Hey @GezimSejdiu
Thank you very much for your contribution! Indeed, having a common layer for feature extractions is really an interesting factorization.
I approve these changes for a merge. 👍

Loading

@GezimSejdiu
Copy link
Member Author

@GezimSejdiu GezimSejdiu commented Jan 8, 2019

Hi @dgraux ,
indeed. Will consider adding basic unit-test for the vandalism detection methods.

Best,

Loading

dgraux
dgraux approved these changes Jan 9, 2019
Copy link
Member

@dgraux dgraux left a comment

Hi @GezimSejdiu
Thank you very much for the unit-test addition.

Loading

GezimSejdiu added 5 commits Jan 23, 2019
It has been disabled as of now since the modeling the data (based on the input dataset) is taking longer.
We are going to provide a smaller dataset which takes less time to run.
@GezimSejdiu GezimSejdiu merged commit 73350b2 into develop Jan 23, 2019
4 checks passed
Loading
@GezimSejdiu GezimSejdiu deleted the feature/refactor-vandalismdetection-package branch Jan 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants