Skip to content
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

Include classes that lack schedules + split off IAP/summer #100

Merged
merged 12 commits into from
Dec 15, 2024
Merged

Conversation

psvenk
Copy link
Member

@psvenk psvenk commented Dec 12, 2024

Fixes #27 and #7. In the scraper, replace the check that a class has a schedule with a check that a class is offered in the current term (with the fall/IAP/spring check done in catalog.py while the academic year check is done in fireroad.py), and add proper support for IAP and summer terms.

Since this explicitly checks for classes to be offered in the current term, it excludes IAP classes when the current term is set to spring (and, in principle, summer classes when the current term is set to fall); this means that merging this PR in its current state would remove IAP classes from Hydrant. I plan on resolving #7 within this PR before marking it as ready for review.

psvenk and others added 9 commits December 14, 2024 16:31
Following the changes to RawClass field names in #37 and #83.
This still excludes classes that are not listed in the current course catalog,
since those are filtered out by catalog.py.
We check that a class is offered in the fall/spring in fireroad.py, and check
that it is offered during this academic year in catalog.py. This does have the
effect of excluding IAP when the term is set to spring, until we properly split
IAP and spring.
This allows capturing both IAP and spring, or summer and fall, in the same
latestTerm.json file; the functionality is currently unchanged as only the
"semester" term (fall/spring) is used.
Since no other fields of latestTerm.json are ever read in the frontend (the
actual term file is used for start/end dates, Monday schedule, etc.).
The pre-semester (IAP or summer) is currently output to latestTermPreSemester,
though (it needs to be renamed to i25, etc.). The scrapers also don't look at
scheduleFall, scheduleIAP, scheduleSpring.
This does mean that the pre-semester file -- currently i25.json -- needs
to be added to the .gitignore (and that this needs to be updated each
semester), but it avoids special-casing the latest pre-semester in the
code (i.e., saying that the penultimate term should be read from
latestPreSemester.json rather than the expected file name).
@psvenk psvenk marked this pull request as ready for review December 14, 2024 22:46
@psvenk psvenk changed the title Include classes that lack schedules Include classes that lack schedules + split off IAP/summer Dec 15, 2024
@psvenk psvenk requested a review from dtemkin1 December 15, 2024 00:05
Copy link
Collaborator

@dtemkin1 dtemkin1 left a comment

Choose a reason for hiding this comment

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

Switching between semesters seem to work, and things seem to be good! I tested it by adding 6.S057 (spring class without a schedule) and 16.810 (IAP class with one schedule time TBD) and it seemed to hold up fine and in line with the current functionality.

public/latest.json
public/i25.json
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there no other way of making sure we don't commit presemester? I feel like this will start getting bloated after a few semesters

Copy link
Member Author

Choose a reason for hiding this comment

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

The main other way that I can think of is to put the latest pre-semester in public/latestPreSemester.json and then to add a special case to src/lib/TermSwitcher.tsx so that the urlName corresponding to the latest pre-semester will read from the file public/latestPreSemester.json.

This would break the correspondence between urlNames and JSON file names that we have now; e.g., https://hydrant.mit.edu/?t=latest loads Spring 2025 while https://hydrant.mit.edu/?t=s25 does not, since the file is latest.json and not yet s25.json.

Alternatively, we could make public/i25.json a symlink to public/latestPreSemester.json, but then the maintenance task on each semester is changed from editing .gitignore to modifying a symlink (which is IMO marginally worse).

Bloat shouldn't become an issue here, since it's only the latest pre-semester that needs to be ignored; when the fall 2025 listings are out, public/i25.json should be committed (along with public/s25.json) and it will be public/m25.json that will need to be ignored.

@dtemkin1
Copy link
Collaborator

dtemkin1 commented Dec 15, 2024

Also, classes with pre-8am times show up in the class listing now but with an "unknown schedule," relevant for issue #86

@psvenk psvenk merged commit a1c50bf into main Dec 15, 2024
3 checks passed
@psvenk psvenk deleted the noschedule branch December 15, 2024 03:46
@psvenk psvenk mentioned this pull request Dec 15, 2024
4 tasks
@gabrc52
Copy link
Contributor

gabrc52 commented Dec 16, 2024

This has caused some "not offered" classes to show up: #110

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.

[Low Priority] What to do with classes missing a schedule
3 participants