-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add script to get usernames of course participants #52
Conversation
Need to add some documentation and a Makefile entry before merging. |
src/get_usernames.py
Outdated
argparser = argparse.ArgumentParser(description='Get usernames of students taking courses this semester') | ||
argparser.add_argument('--username', '-u', help='Username for login', type=str) | ||
argparser.add_argument('--password', '-p', help='Password for login', type=str) | ||
argparser.add_argument('--semester', '-s', help='Semester to read data for', type=str) |
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.
Misleading. You can only get usernames for current semester, so this option only decides where to read/write in local data.
Semester to save data to
or similar.
@evestera Ready for review? |
@olehermanse Sure. Should be good to go. |
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.
Nice. One small request: add an argument for --input-file
.
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.
Tested that this works. Well written. Nice that you included some tests. Merging now, can add the requested feature later.
argparser.add_argument('--username', '-u', help='Username for login', type=str) | ||
argparser.add_argument('--password', '-p', help='Password for login', type=str) | ||
argparser.add_argument('--prev', help='Read data for the previous semester', action='store_true') | ||
args = argparser.parse_args() |
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.
Would be useful to have an optional argument --filename
or --input
if you want to specify a different course list than spring.json
or fall.json
Closes #20
Needed for #21