-
Notifications
You must be signed in to change notification settings - Fork 0
Microtask 4 #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Microtask 4 #4
Conversation
App/views.py
Outdated
'list':'usercontribs', | ||
'uclimit':'5', | ||
'ucuser':username} | ||
url_parameters = urllib.parse.urlencode(parameters) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should just use requests.get('https://en.wikipedia.org/w/api.php', params=parameters)
and requests does the encoding for you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay sir.
App/views.py
Outdated
response = requests.get(url1) | ||
|
||
if response.status_code == 200: | ||
lst = json.loads(response.text) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'd probably want a try...catch around this as well, in case the API returns invalid JSON.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was my fault. Rectified.
App/views.py
Outdated
{"error": "0 edits found for username {}".format(username)}) | ||
|
||
for articles in contributions: | ||
url2 = "https://ores.wikimedia.org/v3/scores/enwiki/" + str(articles['revid']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
YMMV but I find string interpolation more readable than concatenation - you can see the string structure at a glance when skimming the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to string formatting. 👍
App/views.py
Outdated
) | ||
except: | ||
# Edit data extracted | ||
edits_data = json.loads(response.text) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You already parsed this a few lines above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rectified.
App/views.py
Outdated
|
||
if response.status_code == 200: | ||
lst = json.loads(response.text) | ||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a weird way to use exceptions. Normally you would put the normal behavior in the try branch and the error handling in the except branch. In part because it's easier for others to understand your thinking that way, in part because the main branch tends to be longer and involve multiple types of errors. So now you have an except branch which itself can throw exceptions... not a good thing.
Also, specifically for dictionary existence checks, you can just use 'error' in lst
or lst.get('error')
, neither of which can throw exceptions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes sir, it should have been that way. I have corrected it now.
@tgr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
App/views.py
Outdated
# Edit data extracted | ||
edits_data = json.loads(response.text) | ||
if 'error' in edits_data: | ||
return(render(request, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works just fine, but you could also just do something like
if 'error' in edits_data:
raise Error(edits_data['error']['info'])
which is arguably easier to read (all the error rendering happens in one place). As your code becomes more complex, you might want to separate rendering form the main control flow (use mode-view-controller or something similar) and exceptions are one way to handle it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion. I have corrected code to handle all error rendering in except branch.
@tgr
Sir,
This tool provides edit quality and article quality analysis of last 5 edits made by user in english Wiki (using ORES API).
Tool- link
Please review.
Thank you.