Skip to content
This repository has been archived by the owner on Apr 15, 2019. It is now read-only.

Characters PLOD #129

Closed
mina-kleid opened this issue Apr 2, 2016 · 58 comments
Closed

Characters PLOD #129

mina-kleid opened this issue Apr 2, 2016 · 58 comments

Comments

@mina-kleid
Copy link

More documentation for the following character PLOD would be great

  • Get characters PLOD by :algorithm . which algorithms are available and what should i provide in the request for each algorithm also are they going to be sorted ?
  • Get character PLOD values, limited by :count parameter , are they going to be sorted ? if yes Ascending or descending ?
  • https://api.got.show/api/plod/find , how should be the request format? is it in the body or in the header like the character find ?

also currently all PLOD requests doesnt return anything

@mina-kleid mina-kleid changed the title More documentation for Characters PLOD Characters PLOD Apr 2, 2016
@Legenzoo
Copy link
Collaborator

Legenzoo commented Apr 2, 2016

@togiberlin Haven´t you implemented this?

@sacdallago
Copy link
Contributor

There's actually no way to populate the PLOD collection. No NPM script, nor I see the collection on the db.

I NEED full hands on this issue, now.

@kordianbruck @Adiolis @togiberlin @boriside

@sacdallago
Copy link
Contributor

You don't even have the dependency in the package.json?!?!!!!

What the fuck guys, who was responsible for this?????

@sacdallago
Copy link
Contributor

@TheZoker @AlexMoroz @alschm

@sacdallago sacdallago added this to the v0.2.0 Delivery milestone Apr 3, 2016
@Legenzoo
Copy link
Collaborator

Legenzoo commented Apr 3, 2016

Sorry, i can´t really help here, because i am not aware of the things that have been done according the PLOD. I am only as up-to-date as the issue #101.

I also see no possibility to fill the db by scraper or data json. So probably another group used the api @togiberlin implemented to fill the data?

I know that there has been some Plod data on @kordianbruck ´s server.

@sacdallago
Copy link
Contributor

@Adiolis that might have been mock data, because one quick look at the repos of B illuminates us all:

  1. https://github.com/Rostlab/JS16_ProjectB_Group6/blob/develop/main.js that can be imported and called with the character name and the PLOD stored in the relevant collection
  2. https://github.com/Rostlab/JS16_ProjectB_Group7/blob/develop/npm.js same thing.

Literally: Import package, for every character get the prediction via the relevant function in the package, store it in the DB.

@sacdallago
Copy link
Contributor

Actually, let me be even more specific:

  1. https://github.com/Rostlab/JS16_ProjectB_Group6/blob/develop/main.js#L16
  2. https://github.com/Rostlab/JS16_ProjectB_Group7/blob/develop/npm.js#L59

Don't get me wrong, I know you were not responsible for this. I know who was responsible for this. @TheZoker @togiberlin ( in part @CavidSalahov @AlexMoroz )

EDIT I know my chickens

@Legenzoo
Copy link
Collaborator

Legenzoo commented Apr 3, 2016

I notified the group on facebook. I can´t do that before Thursday. =/

@TheZoker
Copy link
Collaborator

TheZoker commented Apr 3, 2016

@sacdallago Which dependency is missing in the package.json? I don't fully understand what's going on right now
Do you mean the hole package of project B? https://www.npmjs.com/package/gotplod

@sacdallago
Copy link
Contributor

@TheZoker https://github.com/Rostlab/JS16_ProjectA/blob/master/package.json#L20
There should be the two packages from B somewhere here, and someone in A should be using them to generate the PLODs

TheZoker added a commit that referenced this issue Apr 3, 2016
@TheZoker
Copy link
Collaborator

TheZoker commented Apr 3, 2016

These are the missing dependencies, right?

@sacdallago
Copy link
Contributor

Step one.

@TheZoker
Copy link
Collaborator

TheZoker commented Apr 3, 2016

Am I responsible for anything else here?

@AlexMoroz
Copy link
Collaborator

@TheZoker use "latest" for package versions

@sacdallago
Copy link
Contributor

@TheZoker no. thanks
but yes, @AlexMoroz is right

@TheZoker
Copy link
Collaborator

TheZoker commented Apr 3, 2016

@AlexMoroz Like "gotarffplod": "latest"?

@AlexMoroz
Copy link
Collaborator

yes

@TheZoker
Copy link
Collaborator

TheZoker commented Apr 3, 2016

Done.

AlexMoroz added a commit that referenced this issue Apr 3, 2016
@sacdallago
Copy link
Contributor

I'm still unhappily waiting.

@Legenzoo
Copy link
Collaborator

Legenzoo commented Apr 3, 2016

I tried to have a look into the package, but node gives me:

Error: Cannot find module 'gotplod'

I have started npm install with the new package.json and also npm install gotplod.
Using the online test tool of npm gives me the same result:
https://tonicdev.com/npm/gotplod

Error: Cannot find module 'gotplod' Error Viewer
Whoops, should we have 'gotplod'?
We should have every package on npm. Sometimes this error is caused by a typo in your require path, but if you think we are actually missing this package, please confirm below and we’ll do our best to fix it as soon as possible!

@TheZoker @AlexMoroz

Edit: The files are in node_modules. Don´t get the problem with require...

@jsalahov
Copy link
Collaborator

jsalahov commented Apr 3, 2016

@Adiolis Fixed the problem with 'gotplod'. Download the new version.

@AlexMoroz
Copy link
Collaborator

@Adiolis try also gotarffplod

@sacdallago
Copy link
Contributor

No. The packe was not deployed to npm

@AlexMoroz
Copy link
Collaborator

Package deployed.

@sacdallago
Copy link
Contributor

Ok, I've updated the thing but now comes another observation:

why is there an imbalance towards the number of predicted characters with one alg and with the otherone? Is this wanted @goldbergtatyana ?

Look at the output of the call: https://api.got.show/api/plod/

If I filter by gotplod I get 1946 hits, if I filter per gotarffplod I get 174.

@Legenzoo
Copy link
Collaborator

Legenzoo commented Apr 4, 2016

@sacdallago Jep, i have noticed many characters with Plod unknown. I skipped them.

@sacdallago
Copy link
Contributor

Thx @Adiolis , let's wait for the answer of @nicoladesocio @goldbergtatyana or anyone involved in the gotarffplod.

It might as well be design, but it's good to know also for @Mina-Zaki who has to implement this in the froentend

@goldbergtatyana
Copy link
Collaborator

@sacdallago @Adiolis very well spotted! this should absolutely not be. The number of characters predicted by Group 6 (gotplod) is 1946 - which is correct, while the number of characters predicted by Group 7 (gotarffplod) is 1939 - this number is wrong.

@sacdallago
Copy link
Contributor

@nicoladesocio @konstantinos-angelo @dan736923 @AlexMoroz fix?

@goldbergtatyana
Copy link
Collaborator

@s-feng apparently wrong prediction results got delivered to Group A. Can you please talk to @nicoladesocio @konstantinos-angelo @dan736923 @AlexMoroz to fix it by providing results of test 5? Thanks so much!

@dan736923
Copy link
Collaborator

@goldbergtatyana This is because I removed some characters with invalid dateOfBirth/Death attributes (e.g. 298299) in test5. (Which also would have messed up age attribute) If you think it is useful, I can make another run including those characters and upload new JSON files.

@AlexMoroz
Copy link
Collaborator

@dan736923 but now only 174 characters have plod

@goldbergtatyana
Copy link
Collaborator

thanks @AlexMoroz for running the test.

@dan736923 but you used all of these features for the prediction of plods for 1939 characters. so, yeah, we do need plod predictions for all of them!

@TheZoker
Copy link
Collaborator

TheZoker commented Apr 4, 2016

@AlexMoroz Do you need help with anything?

@Legenzoo
Copy link
Collaborator

Legenzoo commented Apr 4, 2016

@sacdallago This is no issue anymore for Project A, isn´t it? The filling of Plods and the API works.
I would close this here and well, open an issue on the other project, because to less data is provided.

@Legenzoo Legenzoo closed this as completed Apr 4, 2016
@AlexMoroz
Copy link
Collaborator

@TheZoker no,we've fixed it, thanks

@AlexMoroz
Copy link
Collaborator

gotarffplod@0.0.5 with new predictions is published

@sacdallago
Copy link
Contributor

Niiiice

@Legenzoo
Copy link
Collaborator

Legenzoo commented Apr 5, 2016

@sacdallago

@sacdallago sacdallago assigned sacdallago and unassigned AlexMoroz Apr 5, 2016
@sacdallago
Copy link
Contributor

yeah yeah @Adiolis later, gotta run now :D

@Legenzoo
Copy link
Collaborator

Legenzoo commented Apr 5, 2016

You are not angry anymore, aren´t you? 😆

@sacdallago
Copy link
Contributor

nope, I'm just behind on work... Now it's done.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests