Skip to content

Conversation

@dj0409
Copy link
Contributor

@dj0409 dj0409 commented May 3, 2025

Description

This PR introduces a new feature to generate PDF reports from matched questionnaire items using fpdf. The function generate_pdf_report() creates a structured and readable PDF summarizing matched question pairs across instruments, with optional threshold filtering and match score display.

The report includes:

  • Table-formatted matched question pairs with instrument names and question numbers.

  • Match percentage per pair.

  • A count of matched items in the header.

  • A configurable threshold to filter weaker matches.

A new test file tests/test_export_pdf_report.py was also added to validate the PDF export functionality and the requirements.txt where updated because it needs fpdf.

Fixes # (issue)

Not linked to a specific issue — this is a standalone feature contribution, since i implemented it in python, there is an issue in the harmony app which wants this feature tho ( this would be #53 then)

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Testing

Tests were added in tests/test_export_pdf_report.py:

test_high_threshold_yields_no_matches: Verifies the report correctly shows 0 matches with a 99% threshold.

test_default_threshold_yields_some_matches: Verifies that matches appear with the default 50% threshold.

To run the tests:
pytest -q

Test Configuration

  • Library version: latest main of harmony
  • OS: Windows 10
  • Toolchain: Python 3.11, PyCharm, pytest, venv

Checklist

  • [x ] My PR is for one issue, rather than for multiple unrelated fixes.
  • [x ] My code follows the style guidelines of this project. I have applied a Linter (recommended: Pycharm's code formatter) to make my whitespace consistent with the rest of the project.
  • [x ] I have performed a self-review of my own code
  • [x ] I have commented my code, particularly in hard-to-understand areas
  • [x ] I have made corresponding changes to the documentation
  • [x ] My changes generate no new warnings
  • [x ] I have added tests that prove my fix is effective or that my feature works
  • [x ] New and existing unit tests pass locally with my changes
  • [x ] Any dependent changes have been merged and published in downstream modules
  • [x ] I have checked my code and corrected any misspellings
  • [x ] The Harmony API is not broken by my change to the Harmony Python library
  • [x ] I add third party dependencies only when necessary. If I changed the requirements, it changes in requirements.txt, pyproject.toml and also in the requirements.txt in the API repo
  • [x ] If I introduced a new feature, I documented it (e.g. making a script example in the script examples repository so that people will know how to use it.

Optionally: feel free to paste your Discord username in this format: discordapp.com/users/yourID in your pull request description, then we can know to tag you in the Harmony Discord server when we announce the PR.

@jaydugad
Copy link
Collaborator

jaydugad commented May 6, 2025

Thanks a lot for this contribution! Really solid work, the PDF export feature is cleanly implemented, well-structured, and easy to follow. I ran the tests for both high and default thresholds and they passed without issues. The code is tidy and the docstrings are clear and helpful. I also checked that fpdf is properly added to the requirements and the output PDF looks great, with all the expected metadata included. No changes to the existing API behaviour either, which is perfect. Happy to approve this.

@jaydugad jaydugad merged commit 694a3db into harmonydata:main May 6, 2025
1 check passed
@woodthom2
Copy link
Contributor

Hi @dj0409 thanks so much for this contribution, this is really valuable!

I have a couple of questions

  1. Are both these dependencies necessary? Are they the same thing? I was also wondering about the use of ~, since I have not seen it before.
fpdf~=1.7.2
fpdf2~=2.8.2
  1. Do you have any corresponding change to the API repo? Since if they are added to requirements.txt here we would also need a PR to the API repo: https://github.com/harmonydata/harmonyapi

thanks!

@dj0409
Copy link
Contributor Author

dj0409 commented Jun 2, 2025

hey I'm happy to hear that it works, I saw also regarding the requirements, only fpdf==1.7.2 is necessary not both. And I just did the PR to the API repo thanks :)

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