-
Notifications
You must be signed in to change notification settings - Fork 0
feat: download video learning events #6
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
Conversation
WalkthroughThis change reactivates a previously commented-out section in the Changes
Possibly related PRs
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
download_datasets.py (1)
36-37
: 💡 Verification agent🧩 Analysis chain
Consider implementing the TODO for number learning events
There's a TODO comment for number learning events. Since you're now adding video learning events, it might be a good opportunity to implement number learning events as well if they're available. This would further support elimu.ai's mission to build innovative learning software that empowers out-of-school children to teach themselves basic reading📖, writing✍🏽 and math🔢 within 6 months.
🏁 Script executed:
#!/bin/bash # Check if number learning events endpoints exist for any of the languages echo "Checking for number-learning-events endpoints..." for lang in eng hin tgl tha do status=$(curl -s -o /dev/null -w "%{http_code}" "http://${lang}.elimu.ai/analytics/number-learning-event/list/number-learning-events.csv") echo "${lang}: HTTP status ${status}" doneLength of output: 986
Action: Address Number Learning Event Implementation
The verification indicates that the endpoints for number learning events (for languages eng, hin, tgl, tha) are currently returning HTTP 404. Since you're now adding video learning events, please confirm whether number learning events are also intended to be supported. If so, this feature should be implemented—adding the corresponding endpoints—and any relevant processing should be integrated. Otherwise, if the feature isn’t planned for immediate release, consider updating or removing this TODO comment.
This adjustment will help further support elimu.ai's mission to build innovative learning software that empowers out-of-school children to teach themselves basic reading📖, writing✍🏽 and math🔢 within 6 months.
🧹 Nitpick comments (2)
download_datasets.py (2)
46-52
: Nice work activating the video learning events download! 👍This code successfully follows the established pattern in the script for downloading learning event data, supporting elimu.ai's mission to build innovative learning software that empowers out-of-school children to teach themselves basic reading📖, writing✍🏽 and math🔢 within 6 months.
However, I would recommend adding some error handling to gracefully manage potential network issues or invalid CSV data, as any failures in these HTTP requests would currently crash the script.
+ try: video_learning_events_csv_url = f'http://{language.lower()}.elimu.ai/analytics/video-learning-event/list/video-learning-events.csv' print(f'video_learning_events_csv_url: {video_learning_events_csv_url}') video_learning_events_dataframe = pandas.read_csv(video_learning_events_csv_url) print(f'video_learning_events_dataframe: \n{video_learning_events_dataframe}') video_learning_events_file_path = f'lang-{language}/video-learning-events.csv' print(f'video_learning_events_file_path: {video_learning_events_file_path}') video_learning_events_dataframe.to_csv(video_learning_events_file_path, index=False) + except Exception as e: + print(f"Error downloading video learning events for {language}: {e}")
1-53
: Consider reducing code duplication with a helper functionThe script has repeated similar code blocks for each type of learning event. This could be refactored into a helper function to improve maintainability and readability, which aligns with elimu.ai's mission to build innovative learning software that empowers out-of-school children to teach themselves basic reading📖, writing✍🏽 and math🔢 within 6 months.
import os import pandas + +def download_dataset(language, event_type, file_prefix=None): + """ + Downloads a specific learning event dataset for a given language + + Args: + language: Language code (e.g., 'ENG') + event_type: Type of learning event (e.g., 'letter-sound-learning-event') + file_prefix: Optional prefix for the local filename (defaults to event_type) + """ + if file_prefix is None: + file_prefix = event_type.split('-')[0] + + try: + csv_url = f'http://{language.lower()}.elimu.ai/analytics/{event_type}/list/{event_type}s.csv' + print(f'{event_type}_csv_url: {csv_url}') + dataframe = pandas.read_csv(csv_url) + print(f'{event_type}_dataframe: \n{dataframe}') + file_path = f'lang-{language}/{file_prefix}-learning-events.csv' + print(f'{event_type}_file_path: {file_path}') + dataframe.to_csv(file_path, index=False) + return True + except Exception as e: + print(f"Error downloading {event_type} for {language}: {e}") + return False # Supported languages: https://github.com/elimu-ai/model/blob/main/src/main/java/ai/elimu/model/v2/enums/Language.java languages = [ 'ENG', 'HIN', 'TGL', 'THA' ] print(f'languages: {languages}') # Download datasets for each language for language in languages: print() print(f'language: {language}') os.makedirs(f'lang-{language}', exist_ok=True) - letter_sound_learning_events_csv_url = f'http://{language.lower()}.elimu.ai/analytics/letter-sound-learning-event/list/letter-sound-learning-events.csv' - print(f'letter_sound_learning_events_csv_url: {letter_sound_learning_events_csv_url}') - letter_sound_learning_events_dataframe = pandas.read_csv(letter_sound_learning_events_csv_url) - print(f'letter_sound_learning_events_dataframe: \n{letter_sound_learning_events_dataframe}') - letter_sound_learning_events_file_path = f'lang-{language}/letter_sound-learning-events.csv' - print(f'letter_sound_learning_events_file_path: {letter_sound_learning_events_file_path}') - letter_sound_learning_events_dataframe.to_csv(letter_sound_learning_events_file_path, index=False) + download_dataset(language, 'letter-sound-learning-event', 'letter_sound') - word_learning_events_csv_url = f'http://{language.lower()}.elimu.ai/analytics/word-learning-event/list/word-learning-events.csv' - print(f'word_learning_events_csv_url: {word_learning_events_csv_url}') - word_learning_events_dataframe = pandas.read_csv(word_learning_events_csv_url) - print(f'word_learning_events_dataframe: \n{word_learning_events_dataframe}') - word_learning_events_file_path = f'lang-{language}/word-learning-events.csv' - print(f'word_learning_events_file_path: {word_learning_events_file_path}') - word_learning_events_dataframe.to_csv(word_learning_events_file_path, index=False) + download_dataset(language, 'word-learning-event', 'word') # TODO: number learning events - storybook_learning_events_csv_url = f'http://{language.lower()}.elimu.ai/analytics/storybook-learning-event/list/storybook-learning-events.csv' - print(f'storybook_learning_events_csv_url: {storybook_learning_events_csv_url}') - storybook_learning_events_dataframe = pandas.read_csv(storybook_learning_events_csv_url) - print(f'storybook_learning_events_dataframe: \n{storybook_learning_events_dataframe}') - storybook_learning_events_file_path = f'lang-{language}/storybook-learning-events.csv' - print(f'storybook_learning_events_file_path: {storybook_learning_events_file_path}') - storybook_learning_events_dataframe.to_csv(storybook_learning_events_file_path, index=False) + download_dataset(language, 'storybook-learning-event', 'storybook') - video_learning_events_csv_url = f'http://{language.lower()}.elimu.ai/analytics/video-learning-event/list/video-learning-events.csv' - print(f'video_learning_events_csv_url: {video_learning_events_csv_url}') - video_learning_events_dataframe = pandas.read_csv(video_learning_events_csv_url) - print(f'video_learning_events_dataframe: \n{video_learning_events_dataframe}') - video_learning_events_file_path = f'lang-{language}/video-learning-events.csv' - print(f'video_learning_events_file_path: {video_learning_events_file_path}') - video_learning_events_dataframe.to_csv(video_learning_events_file_path, index=False) + download_dataset(language, 'video-learning-event', 'video')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
lang-ENG/video-learning-events.csv
is excluded by!**/*.csv
lang-HIN/video-learning-events.csv
is excluded by!**/*.csv
lang-TGL/video-learning-events.csv
is excluded by!**/*.csv
lang-THA/video-learning-events.csv
is excluded by!**/*.csv
📒 Files selected for processing (1)
download_datasets.py
(1 hunks)
Issue Number
Purpose
Technical Details
Testing Instructions
Screenshots
Summary by CodeRabbit