Skip to content

Conversation

@davidsbatista
Copy link
Collaborator

@davidsbatista davidsbatista commented May 20, 2025

Overview

  • Improving code structure and maintainability through refactoring.
  • No original code was changed - the changes are purely structural, moving existing functionality into a more organized class-based architecture.

Key Changes

Code Restructuring

  • Refactored evaluation logic into proper classes
  • Moved existing data loading methods into dedicated loader classes
  • Integrated reporting methods as class methods

Code Quality Improvements

  • Added comprehensive test coverage
  • Implemented better error handling
  • Enhanced code documentation (docstrings)
  • Added some type hints

Verification

The compare_versions.py script has been added to verify that the old and new code produce identical output content. While the results are functionally equivalent, the new code provides better visualisation for the reporting based on indices

Testing

  • All existing tests pass
  • New test cases added for new code

@davidsbatista davidsbatista marked this pull request as ready for review May 28, 2025 15:00
@davidsbatista
Copy link
Collaborator Author

davidsbatista commented May 28, 2025

Hi @ivyleavedtoadflax! I did a big refactoring on this code - I think it deserved it. I still kept the old functionality.

I suggest we make a release with some warnings for upcoming breaking changes - and after 1 or 2 weeks we release this new version. What do you think?

@ivyleavedtoadflax
Copy link
Collaborator

Great @davidsbatista, yes I think it deserved it. I will give this a review, but agree with the idea of a release warning about breaking changes in the meantime

Copy link
Collaborator

@ivyleavedtoadflax ivyleavedtoadflax left a comment

Choose a reason for hiding this comment

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

Looks, good, much tidier! LGTM 🥳

@ivyleavedtoadflax
Copy link
Collaborator

@davidsbatista looks good, happy to approve. Let's go ahead with the warning about breaking changes in the meantime, and you can approve once we've given a couple of weeks warning.

@davidsbatista
Copy link
Collaborator Author

@ivyleavedtoadflax adding deprecation warnings here

@davidsbatista
Copy link
Collaborator Author

removing old code, so that only the new version works - @ivyleavedtoadflax let me know when you release the current main which has the warnings merged

@ivyleavedtoadflax
Copy link
Collaborator

@ivyleavedtoadflax let me know when you release the current main which has the warnings merged

Done

@davidsbatista
Copy link
Collaborator Author

somehow pip index is not showing the last release, only one from last year is availalbe

@davidsbatista davidsbatista merged commit aa16823 into main Jun 7, 2025
3 checks passed
@davidsbatista davidsbatista deleted the refactoring branch June 7, 2025 10:20
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