Skip to content

DPI and dark theme updates#33

Closed
michael-rutherford wants to merge 4 commits intoRSNA:masterfrom
michael-rutherford:master
Closed

DPI and dark theme updates#33
michael-rutherford wants to merge 4 commits intoRSNA:masterfrom
michael-rutherford:master

Conversation

@michael-rutherford
Copy link
Collaborator

No description provided.

@mdevans
Copy link
Collaborator

mdevans commented Oct 1, 2025

Hi Michael,

Thank you very much for getting onboard, I sincerely hope you found the code learning curve reasonable and structure sensible.

Re. your changes above, first two look good, good idea to control the html text color via the current theme and I'm not sure where those dependency list items went.
Re. adding a separate theme file for dark mode, I don't think this is necessary, the existing theme file has two colors for each item for light and dark mode respectively, as per the customtkinter theme file spec. Is there any specific reason you created a separate file?
Re. windows DPI awareness, thanks for finding that ctypes.windll call, please could you rather use `if sys.platform.startswith("win")' as per line 76 src/anonymizer/anonymizer.py instead of relying on exception, also please can you run the unit tests to ensure formatting & linting checks are passed before committing to repo, you can check the test run under Actions to see how your code failed.

I'd love to hear of you experiences using the Anonymizer ...

Michael

@mdevans mdevans closed this Oct 1, 2025
@michael-rutherford
Copy link
Collaborator Author

Apologies. I was being dumb on with the theme file. I'm more back-end dev than front and new to customtkinter. I'll fix and push again.

@michael-rutherford
Copy link
Collaborator Author

=========================== short test summary info ============================
FAILED tests/controller/test_aws_upload.py::test_send_1_dicomfile_to_AWS_S3_and_list_objects - assert None
====== 1 failed, 143 passed, 5 skipped, 71 warnings in 144.73s (0:02:24) =======

I can't get past this one failure without proper credentials. I set up a cognito user pool with a testing user, but no luck.

Suggestions?

@mdevans
Copy link
Collaborator

mdevans commented Oct 1, 2025

You are running locally? That test sends a dicom file to the RSNA S3 bucket, I will need to send you a test user for their system, please email me.

@mdevans
Copy link
Collaborator

mdevans commented Oct 1, 2025

... although it should work if you setup your own S3 bucket and Cognito user pool, however there are some authorisation subtleties courtesy of AWS & boto which make it tricky

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