Skip to content

Conversation

@Shizoqua
Copy link

Summary

This PR fixes the "new" badge logic in the model gallery endpoints by comparing real dates instead of doing string comparisons on "added".

Problem

/model/gallery and /model/model_groups_list previously computed "new" using model["added"] > cutoff where both values were strings (e.g. "2024-12-01"). This is fragile and can produce incorrect results depending on formatting (and it’s explicitly marked with a TODO in the code).

Changes

  • Parse "added" into a datetime.date safely (with fallback when missing/invalid).
  • Compute "new" using date comparison.
  • Added tests covering:
    • valid ISO dates
    • invalid "added" values
    • missing "added"

Endpoints affected

  • GET /model/gallery
  • GET /model/model_groups_list

Testing

Added/updated pytest coverage in api/test/api/test_model.py.

@josh-janes
Copy link
Contributor

@Shizoqua Hey thanks for looking into this! We will review and get back to you shortly

Copy link
Member

@deep1401 deep1401 left a comment

Choose a reason for hiding this comment

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

Hi @Shizoqua,
Thanks for your contribution!
I tested your code and everything looks good!
We just require ruff formatting to be applied to any code that we merge. If you follow the below mentioned steps, you should be able to apply the formatting and we can approve the PR then. Please run this in your local transformerlab-app clone only so it only formats files in there. Run this from the repository root (transformerlab-app) only.

pip install ruff
ruff format 

It also looks like the pytests are failing, you may want to fix that:

=========================== short test summary info ============================
FAILED test/api/test_model.py::test_model_gallery_new_badge_uses_dates - TypeError: cannot set 'today' attribute of immutable type 'datetime.date'
FAILED test/api/test_model.py::test_model_group_gallery_new_badge_uses_dates - TypeError: cannot set 'today' attribute of immutable type 'datetime.date'

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.

3 participants