Skip to content

Improve API responses and marshal scraper data#80

Closed
madhavarshney wants to merge 6 commits intomasterfrom
feat/improve-api-and-marshal-data
Closed

Improve API responses and marshal scraper data#80
madhavarshney wants to merge 6 commits intomasterfrom
feat/improve-api-and-marshal-data

Conversation

@madhavarshney
Copy link
Copy Markdown
Member

@madhavarshney madhavarshney commented Jul 28, 2020 •
edited
Loading

Key Changes:

  • The structure of a few API responses have been changed to make them less nested and more usable.
  • The class schema has been completely changed. Check owl_models.py.
    • The major change is changing it from an array of classes with times - [ {}, {} ] - to an object with a time array - { time: [] }.
  • Data scraped is now validated and marshalled into a proper schema. For example, units is now automatically converted from a string to a double. This is done through marshmallow.

Future TODOs:

  • Fix server error responses to make them JSON compliant
  • Change DB structure to make it less nested (Migration to new data pattern #63)
  • Use the same data marshalling in the advanced scraper

@madhavarshney madhavarshney added the enhancement New feature or request label Jul 28, 2020
@madhavarshney madhavarshney force-pushed the feat/improve-api-and-marshal-data branch from 4fdab20 to a4aaa3f Compare July 28, 2020 08:23
Base automatically changed from feat/snapshot-tests to master July 29, 2020 00:13
@madhavarshney madhavarshney force-pushed the feat/improve-api-and-marshal-data branch 2 times, most recently from 2d2a6f2 to c02ad04 Compare July 29, 2020 01:35
madhavarshney
madhavarshney commented Jul 29, 2020
View reviewed changes
@madhavarshney madhavarshney requested a review from phi-line July 29, 2020 08:50
madhavarshney
madhavarshney commented Jul 29, 2020
View reviewed changes
owl_models.py
@@ -0,0 +1,35 @@
from marshmallow import Schema, fields, validate, EXCLUDE

class ClassDataSchema(Schema):
Copy link
Copy Markdown
Member Author

@madhavarshney madhavarshney Jul 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

TODO:

  • Add descriptions
  • Update docs

madhavarshney
madhavarshney commented Jul 29, 2020
View reviewed changes
server.py
data[f'{dept}'].append({k: generate_url(dept, k) for k in keys})

return jsonify(data), 200
return jsonify({k: {kk: vv for d in v for kk, vv in d.items()} for k, v in data.items()}), 200
Copy link
Copy Markdown
Member Author

@madhavarshney madhavarshney Jul 29, 2020

Choose a reason for hiding this comment

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

TODO: simplify 😄

Copy link
Copy Markdown
Collaborator

@phi-line phi-line Jul 29, 2020

Choose a reason for hiding this comment

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

ah yes one liners :D
love em and hate em at the same time

Copy link
Copy Markdown
Member Author

@madhavarshney madhavarshney Jul 30, 2020

Choose a reason for hiding this comment

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

To be honest, I can never really understand Python one-liners (though I half-wrote this, half SO'd it).. maybe I just need more experience with them. =P

@phi-line
Copy link
Copy Markdown
Collaborator

phi-line commented Jul 29, 2020

Just a note.

We might want to just build a schedule maker as part of the API. Like requesting courses and getting a generated list of all possible schedules possible as a response

@madhavarshney
Copy link
Copy Markdown
Member Author

madhavarshney commented Jul 30, 2020

@phi-line Possibly. For this PR, take a look at my comment on key names above. What do you think about those? Additionally, although this is trivial, should the "times" array be called time or times (or something else altogether)?

@madhavarshney madhavarshney marked this pull request as ready for review July 30, 2020 11:27
@madhavarshney madhavarshney force-pushed the feat/improve-api-and-marshal-data branch from c02ad04 to a9aff8e Compare July 31, 2020 03:34
@madhavarshney madhavarshney force-pushed the feat/improve-api-and-marshal-data branch from a9aff8e to 199719b Compare August 10, 2020 06:11
@madhavarshney madhavarshney changed the base branch from master to fix/75-course-string-parser August 10, 2020 06:11
@madhavarshney madhavarshney mentioned this pull request Aug 10, 2020
Merged
Base automatically changed from fix/75-course-string-parser to master August 10, 2020 19:49
@madhavarshney madhavarshney force-pushed the feat/improve-api-and-marshal-data branch from 199719b to c231a58 Compare August 10, 2020 19:58
@madhavarshney
feat: improve data format - init work
2142d5e 
- Change course model structure
- Properly marshal data (str -> int)
@madhavarshney
feat: fix cors, improve API responses
dab55ff
@madhavarshney
feat: improve data format - more changes
f5614ff 
- Add documention to course model
- Rename and improve some course keys
@madhavarshney
feat: add model tests
cdcee76
@madhavarshney madhavarshney force-pushed the feat/improve-api-and-marshal-data branch from c231a58 to 9319439 Compare August 31, 2020 20:33
@madhavarshney madhavarshney force-pushed the feat/improve-api-and-marshal-data branch from 9319439 to 20f0d2b Compare September 1, 2020 03:32
@madhavarshney
Copy link
Copy Markdown
Member Author

madhavarshney commented Nov 21, 2020 •
edited
Loading

This will no longer be merged because of its significant number of breaking changes. They have been included in the new OpenCourseAPI.

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

Reviewers

@phi-line phi-line Awaiting requested review from phi-line

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@madhavarshney @phi-line