422 - Setting up pixl locally #423

mxochicale · 2024-07-17T13:56:05Z

Addressing #422

Mainly, I am trying to setup pixl on my local host device, so I am adding few changes.

These are some notes that came to mind when looking pixl repo:

README looks great and migth be improving by linking relevant sections to avoid duplication (dev and design sections can be moved to https://github.com/UCLH-Foundry/PIXL/tree/main/cli)
Do we want to remove versions of deps in https://github.com/UCLH-Foundry/PIXL/blob/main/cli/pyproject.toml (core does not have a version)?
Duplicated requirements for test and dev in https://github.com/UCLH-Foundry/PIXL/blob/main/cli/pyproject.toml and https://github.com/UCLH-Foundry/PIXL/blob/main/pixl_core/pyproject.toml and other pixl_* packages
Duplication of setups instructions:

pytest requires debugging as requires other deps to be run

E   ModuleNotFoundError: No module named 'pixl_imaging'
E   ModuleNotFoundError: No module named 'pixl_dcmd'
E   ModuleNotFoundError: No module named 'nibabel'
2 warnings, 6 errors in 0.58s

codecov · 2024-07-17T13:58:02Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.72%. Comparing base (8772612) to head (037c9b7).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #423   +/-   ##
=======================================
  Coverage   83.72%   83.72%           
=======================================
  Files          83       83           
  Lines        3489     3489           
=======================================
  Hits         2921     2921           
  Misses        568      568

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

stefpiatek

Going to wait until #420 is merged before looking at this one, as there's a fair bit of overlap and I think it will also solve your issues

stefpiatek

Some early feedback before #420 is merged

.github/ISSUE_TEMPLATE/not_working.md

.github/ISSUE_TEMPLATE/bug_report.md

.github/ISSUE_TEMPLATE/feature_request.md

cli/README.md

* missing packages paths for pip install, * clone repo details, * re-arrenging dev section in cli/README * adds details to run pre-commit

…ing), and PR template with a list of bits to complete before sending PR review #422

…g to passed `pytest -vs tests/test_docker_commands.py` in OS/Arch: linux/amd64 #422

…tes bug #422

…tivte you python virtual environment` #422

…with links updates #422

…tests/test_message_processing.py #422

p-j-smith

having a PR template and bug report template is a good idea. A few minor comments - some of the items in the PR template checklist are already enforced by required CI checks, and there are a few commands in the docs that error

.github/pull_request_template.md

docs/setup/developer.md

cli/README.md

docs/setup/developer.md

cli/tests/conftest.py

… adding changes and updates of the python version3.11 (matching ci python version), re-order expected message for csv test #422

and CONTRIBUTING.md #425

…nstallation options for `zhs`, adds a more generic instructions to setup venv, and clarity on the use of conda deactivate #422

stefpiatek

Looks good, thanks for getting this in! Happy for you to merge once you've addressed all of the comments to your satisfaction

.github/pull_request_template.md

CONTRIBUTING.md

cli/README.md

docs/setup/developer.md

…tribution guilelines #425

p-j-smith

looking good! Just one reference to https://github.com/UCL/real-time-ai-for-surgery that should be removed before merging

CONTRIBUTING.md

…ref and proving the option of rebase if need it (which is something I find it usuefull in many of other projects) #422

…ibution guideline, correction of github organistaion and reponame #422

…rflows, adds meaning of VE #422

…other minor and related changes #422

mxochicale · 2024-07-22T15:06:35Z

Thanks @stefpiatek and @p-j-smith for your comments and feedback, I think I have addressed the majority of your comments. Let me know if there is anything I might have missed, otherwise I will merge this one tomorrow morning.

stefpiatek reviewed Jul 17, 2024

View reviewed changes

stefpiatek reviewed Jul 18, 2024

View reviewed changes

mxochicale added 6 commits July 18, 2024 13:05

few changes on setting up pixl #422, including:

8fcfc60

* missing packages paths for pip install, * clone repo details, * re-arrenging dev section in cli/README * adds details to run pre-commit

adds github templates for issues (bugs, feature requests and not work…

e8051ed

…ing), and PR template with a list of bits to complete before sending PR review #422

adds versions for Docker Compose in README and details to run, leadin…

a89c11e

…g to passed `pytest -vs tests/test_docker_commands.py` in OS/Arch: linux/amd64 #422

migrate md to yml issues templates and removes not_working as dublica…

de53f70

…tes bug #422

removes conda commands in favour of being more generalistic using `ac…

a28242f

…tivte you python virtual environment` #422

re-organising location of python virtual envs to docs/setup/dev... …

33f53b0

…with links updates #422

mxochicale force-pushed the 422-setting-up-pixl-locally branch from 917f8c5 to 33f53b0 Compare July 18, 2024 12:17

few minor changes to devs instructions #422

61bdbd6

mxochicale requested review from a team and stefpiatek July 18, 2024 12:34

fix AttributeError: module datetime has no attribute UTC to pass …

2f7ee59

…tests/test_message_processing.py #422

p-j-smith requested changes Jul 19, 2024

View reviewed changes

p-j-smith reviewed Jul 19, 2024

View reviewed changes

cli/tests/conftest.py Outdated Show resolved Hide resolved

🎉 17 passed cli/tests with local host device under ubuntu22.04 x64 by…

e6840d5

… adding changes and updates of the python version3.11 (matching ci python version), re-order expected message for csv test #422

mxochicale mentioned this pull request Jul 19, 2024

We need a CONTRIBUTING doc #425

Closed

mxochicale added 2 commits July 19, 2024 14:40

changes to .github/pull_request_template.md, adding CODE_OF_CONDUCT #422

b55368d

and CONTRIBUTING.md #425

adds details on correct comman to run tests, adds quotes in dev pip i…

c86cc56

…nstallation options for `zhs`, adds a more generic instructions to setup venv, and clarity on the use of conda deactivate #422

mxochicale requested a review from a team July 19, 2024 14:01

stefpiatek approved these changes Jul 22, 2024

View reviewed changes

stefpiatek requested a review from p-j-smith July 22, 2024 09:36

adds more clarity on the PR template #422 and move local tests to con…

43d3337

…tribution guilelines #425

p-j-smith approved these changes Jul 22, 2024

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

mxochicale added 5 commits July 22, 2024 14:46

adding and recommeding Squashing and merging a long-running branch …

16c1f6d

…ref and proving the option of rebase if need it (which is something I find it usuefull in many of other projects) #422

adds step 6 in PR workflow instead of leaving at the end of the contr…

dbfe204

…ibution guideline, correction of github organistaion and reponame #422

adds the accidental removed pytnon -m in one my previous rebases wo…

e30ee05

…rflows, adds meaning of VE #422

adds explicit details with examples for github workflow pipeline and …

01c71b1

…other minor and related changes #422

adds more details on removing and deleting merged branches #422

4ce2a2e

mxochicale requested a review from a team July 22, 2024 14:57

add empty spaces to create new lines for steps in 9, 9.1, 9.2 #422

037c9b7

mxochicale merged commit 282f64b into main Jul 23, 2024
10 checks passed

mxochicale deleted the 422-setting-up-pixl-locally branch July 23, 2024 08:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

422 - Setting up pixl locally #423

422 - Setting up pixl locally #423

mxochicale commented Jul 17, 2024

codecov bot commented Jul 17, 2024 •

edited

Loading

stefpiatek left a comment

stefpiatek left a comment

p-j-smith left a comment

stefpiatek left a comment •

edited

Loading

p-j-smith left a comment

mxochicale commented Jul 22, 2024

422 - Setting up pixl locally #423

422 - Setting up pixl locally #423

Conversation

mxochicale commented Jul 17, 2024

Addressing #422

codecov bot commented Jul 17, 2024 • edited Loading

Codecov Report

stefpiatek left a comment

Choose a reason for hiding this comment

stefpiatek left a comment

Choose a reason for hiding this comment

p-j-smith left a comment

Choose a reason for hiding this comment

stefpiatek left a comment • edited Loading

Choose a reason for hiding this comment

p-j-smith left a comment

Choose a reason for hiding this comment

mxochicale commented Jul 22, 2024

codecov bot commented Jul 17, 2024 •

edited

Loading

stefpiatek left a comment •

edited

Loading