Skip to content

Conversation

@narendrakumar9867
Copy link
Contributor

@narendrakumar9867 narendrakumar9867 commented Aug 5, 2025

Description

Implements human-readable PDF reports with visualizations as alternative to Excel matrix output. Includes statistics, charts, and maintains backward compatibility.

Fixes # (issue)

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 60% threshold.

To run the tests:
pytest -q

Test Configuration

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

Checklist

  • Implement enhanced harmonisation reports as requested in issue.
  • Add match distribution charts and cross instrument heatmaps
  • Include executive summary with key harmonisation metrics
  • Maintain backward compatibility with existing generate_pdf_export
  • Add comprehensive test suite and error handling
  • My changes generate no new warnings
  • 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.

@narendrakumar9867
Copy link
Contributor Author

@woodthom2 Sir, please review the pr.
Let me know if you have any questions or need any clarifications.

@jaydugad
Copy link
Collaborator

jaydugad commented Aug 8, 2025

Hi @narendrakumar9867,
Thanks for your contribution! While running the tests, I encountered a failure due to a NameError: name 'FPDF' is not defined in tests/test_export_pdf_report.py. This error is causing the build to fail.

Could you please take a look and fix the missing import or any related issue so that the tests pass smoothly? Once done, let me know and I’ll review again.

If you need to discuss anything, feel free to contact me on Discord - my name is Jay Dugad (jaydugad3382).

@narendrakumar9867
Copy link
Contributor Author

narendrakumar9867 commented Aug 9, 2025

Screenshot 2025-08-09 100115 Thank you @jaydugad for reviewing the PR. I have fixed the issue now please check it again.

@jaydugad
Copy link
Collaborator

jaydugad commented Aug 9, 2025

Looks good now, all tests pass. I’ll merge this in — nice work!

@jaydugad jaydugad merged commit 743ed30 into harmonydata:main Aug 9, 2025
1 check passed
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.

2 participants