Skip to content

Conversation

chbeltz
Copy link
Contributor

@chbeltz chbeltz commented Mar 11, 2017

No description provided.

@chbeltz chbeltz changed the title Trust score Trust scores Mar 11, 2017
@weex
Copy link
Contributor

weex commented Mar 12, 2017

@gutsal-arsen do you have any concerns about this PR itself? If you're saying we should do the things you mentioned but not this, I'm open to arguments here. Otherwise if it'll take significant discussion please create an issue and give an idea of the benefits of either of those platforms.

@aahutsal
Copy link

Just replied to you with email @weex. No problems with that PR as I don't read python code ;)

Copy link
Contributor

@weex weex left a comment

Choose a reason for hiding this comment

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

This is 1 to 5, no? Also, mediators can't yet rate anyone due to our bug with them not seeing the jobs they're involved in.

rein/cli.py Outdated
]
trust_links.append(min(trust_values))

dest_trust_score = json.dumps({'score': float(sum(trust_links)) / float(len(trust_links)), 'links': len(trust_links)})
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's wrap this line.

rein/cli.py Outdated
vouched_users = rated_by_source + [(source_msin, 5)]
# Calculate trust links between source and dest
trust_links = []
for vouched_user in vouched_users:
Copy link
Contributor

Choose a reason for hiding this comment

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

Two questions. Does this follow more than two links and if so how does it avoid cyclic ratings? The other question is if you can describe how this works with some different scenarios? I drew this as an example http://imgur.com/PbnaFLf where it would be interesting to know how D's trust score and links should look for A, if I have A and D right in that example as source and destination. Since this can be tricky and important code we definitely need to think about a kind of test with an example graph.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It currently doesn't follow more than two links because I didn't see that implemented in bitcoin otc.

Would you like that? If yes, we'd probably need a hard limit for how many links it should follow at most. Cyclic ratings could probably easily be avoided by keeping track of all source msins.

Some tests definitely seem sensible, I'll think of something. That example is fairly simple though. Because chains are one-sided there is only one link (B) between A and D, giving D a trust score of 4.

<hr>

<h3>Trust score</h3>
<p>Would you like to automatically trust scores for users on their ratings page? If so, check the box below. This may make ratings pages slower to load.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing a word I think, display? calculate?

@chbeltz
Copy link
Contributor Author

chbeltz commented Mar 13, 2017

Yes, it currently is 0 to 5. Did you have something else in mind?

@weex
Copy link
Contributor

weex commented Mar 13, 2017

I thought zero stars was not possible. Typically 1 is the lowest but maybe 0 means no rating? I'd like 1 to 5 at least when a rating is submitted.

@weex
Copy link
Contributor

weex commented Mar 13, 2017

The other stuff about the links is OK for now. I would like to see a basic test though of a small graph.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 9388dfa on FreakJoe:trust-score into ** on ReinProject:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 9db308c on FreakJoe:trust-score into ** on ReinProject:master**.

@weex weex merged commit 259c8ac into ReinProject:master Mar 13, 2017
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

Successfully merging this pull request may close these issues.

4 participants