feat: Parse and display recent contributors from YAML #23
Conversation
- Added Contributor Django model mirroring pyosMeta pydantic models for future database migration - Created YAML parsing utilities to fetch contributor data from remote GitHub source - Updated home view to display 4 most recent contributors with avatars and social links - Added comprehensive error handling and graceful fallbacks - Included PyYAML dependency for YAML parsing
There was a problem hiding this comment.
hey @Phinart98 ! Thank you for this pr!
Please note that i haven't reviewed your code yet but I worry a bit about pyyaml as a dependency as it hasn't had a release in a year or so.
in pyosmeta, we use ruemal-yaml which has had releases as recently as this week! can you please consider updating the dependency to one that is maintainers more actively? In generally this is a good skill to develop in open source! there are SO MANY projects on PyPI and often projects become unmaintained. I like to check last commit date, look at the GitHub repository, etc to figure out if a project is maintained actively or not.
core/utils.py
Outdated
| # Convert to string if it's not already | ||
| date_str = str(date_str).strip() | ||
|
|
||
| # Try common date formats |
There was a problem hiding this comment.
I'm curious about this - are we parsing dates rom the yaml file? If we are, then pyosmeta has already cleaned up the date in that file and it should be consistent (but perhaps you saw something that I missed?)
There was a problem hiding this comment.
@Phinart98 did you have any insight into this?
core/utils.py
Outdated
| cleaned = {} | ||
|
|
||
| # Required field | ||
| if 'github_username' not in contributor: |
There was a problem hiding this comment.
@banesullivan I'm curious what you think about this here. I worry about trying to clean data in multiple places vs handling cleaning when we parse the data over in pyosMeta.
@Phinart98, we have a workflow that generates that YAML file, and it uses helpers in pyosMeta to clean the data. I just think we should do that once - rather than twice UNLESS there is a good reason to do it here.
- Updated function docstrings in core/utils.py to use Parameters/Returns sections - Updated model property docstrings in core/models.py - Updated view docstring in core/views.py - Updated template tag docstrings in core/templatetags/image_tags.py - Updated blog model docstrings in blog/models.py Follows numpy docstring conventions as requested for pyOpenSci project consistency.
|
This pr can be merged when
Melissa you are Phillip or Carol or Bane can all merge when it's ready!! |
- Update dependency from PyYAML>=6.0 to ruamel.yaml>=0.18.0 - Replace yaml.safe_load() with YAML(typ='safe').load() - Update exception handling from yaml.YAMLError to YAMLError
|
Hello @melissawm , I have resolved the merge conflicts and updated the dependency. I believe you can review and hopefully merge now😅 |
There was a problem hiding this comment.
Hi @Phinart98 - not a django person so I didn't look too closely at the code, but I left a couple of general comments. Let me know if anything is not clear. Cheers!
core/admin.py
Outdated
| from django.contrib import admin | ||
|
|
||
| from django.contrib import admin | ||
| # Register your models here. |
There was a problem hiding this comment.
Is this comment supposed to be here? Also, whenever possible, it would be best to add an extra empty line at the end of each file (that will get rid of the red symbols you see here)
There was a problem hiding this comment.
Those comments are there in the starter files when you install Django. I should have cleared them after I added my code. I will do that in my next commit.
I have always wondered why many files I see in repos online have an extra empty line at the bottom. Thanks for pointing this out to me😅
core/utils.py
Outdated
| # Convert to string if it's not already | ||
| date_str = str(date_str).strip() | ||
|
|
||
| # Try common date formats |
There was a problem hiding this comment.
@Phinart98 did you have any insight into this?
- Add comment explaining recent_contributors usage in template - Remove redundant data cleaning since pyosMeta handles it - Simplify contributor sorting to match existing approach - Remove unused imports (datetime, Optional) - Clean up admin.py by removing default Django comment
…com/Phinart98/pyopensci-django into pyOpenSci#10-parse-contributor-data
|
Hello @melissawm , I have worked on the issues raised and pushed another commit. I'm looking forward to hearing your feedback.😊 |
There was a problem hiding this comment.
This looks good to me. If there are any improvements to be made, we can do them in follow-ups.
Thanks @Phinart98 !
closes #10