Refactor the renee python package and move the GUI here #94

kelly-sovacool · 2024-01-16T03:03:20Z

Changes

Moves the renee GUI from the ccbrpipeliner repo to this one (see src/renee/gui.py). The launch_gui() function (previously the main function of the GUI script in ccbrpipeliner) calls run() directly instead of using subprocess. Now the gui is a subcommand of renee and is launched by running renee gui. Various static global variables and environmental variables are now setup dynamically with helper functions.

Also:

Breaks up some of the functions from __main__.py to separate modules to prevent circular imports.
Dynamically sets the default singularity sif cache depending on the HPC.

testing

To test the GUI, switch to this branch and run ./bin/renee gui.

Issues

Towards https://github.com/CCBR/ccbrpipeliner/issues/28
Resolves https://github.com/CCBR/ccbrpipeliner/issues/29
Resolves Use shared singularity cache dir on biowulf and frce #53

PR Checklist

(~~Strikethrough~~ any points that are not applicable.)

This comment contains a description of changes with justifications, with any relevant issues linked.
Update docs if there are any API changes.
Update CHANGELOG.md with a short description of any user-facing changes and reference the PR number. Guidelines: https://keepachangelog.com/en/1.1.0/

copied from renee/main.py in ccbrpipeliner repo

…iner/src

kelly-sovacool · 2024-08-02T18:37:24Z

need to wait for package structure changes from #139 before we can continue with this

Update: it's merged now

to prevent circular imports and make the code easier to maintain

source from CCBR/CHAMPAGNE#180

rather than src/renee/__main__.py, so that the src files will be in the path via ./main.py

resolves #53

kopardev · 2024-08-05T16:37:45Z

@samarth8392 .. Looks good to me ... can you please re-look and approve?

samarth8392 · 2024-08-05T20:14:57Z

I tried testing the gui using ./bin/renee gui and doing a dryrun. I get the following error.

Take a look here:

RENEE/src/renee/gui.py

Line 178 in 8247801

tmp_dir=get_tmp_dir(),

/data/CCBR_Pipeliner/db/PipeDB/Conda/envs/py311/lib/python3.11/tkinter/__init__.py:1531: ResourceWarning: unclosed <socket.socket fd=5, family=2, type=1, proto=0, laddr=('0.0.0.0', 0)>
  for k, v in cnf.items():
ResourceWarning: Enable tracemalloc to get the object allocation traceback
window created!
--SUBMIT-- {'--INDIR--': '/data/mathurs2/input', 'Browse': '/data/mathurs2/input', '--OUTDIR--': '/data/mathurs2/output', 'Browse0': '', '--ANNOTATION--': 'hg38_45'}
['/data/mathurs2/input/sample1-tumor.R2.fastq.gz', '/data/mathurs2/input/sample1-normal.R2.fastq.gz', '/data/mathurs2/input/sample1-normal.R1.fastq.gz', '/data/mathurs2/input/sample1-tumor.R1.fastq.gz']
Traceback (most recent call last):
  File "/vf/users/CCBR/projects/techDev/RENEE/main.py", line 16, in <module>
    sys.exit(main())
             ^^^^^^
  File "/vf/users/CCBR/projects/techDev/RENEE/src/renee/__main__.py", line 1479, in main
    args.func(args)
  File "/vf/users/CCBR/projects/techDev/RENEE/src/renee/gui.py", line 178, in launch_gui
    tmp_dir=get_tmp_dir(),
            ^^^^^^^^^^^^^
TypeError: get_tmp_dir() missing 2 required positional arguments: 'tmp_dir' and 'outdir'

kelly-sovacool · 2024-08-05T21:58:05Z

I tried testing the gui using ./bin/renee gui and doing a dryrun. I get the following error.

Take a look here:

RENEE/src/renee/gui.py

Line 178 in 8247801

tmp_dir=get_tmp_dir(),

/data/CCBR_Pipeliner/db/PipeDB/Conda/envs/py311/lib/python3.11/tkinter/__init__.py:1531: ResourceWarning: unclosed <socket.socket fd=5, family=2, type=1, proto=0, laddr=('0.0.0.0', 0)>
  for k, v in cnf.items():
ResourceWarning: Enable tracemalloc to get the object allocation traceback
window created!
--SUBMIT-- {'--INDIR--': '/data/mathurs2/input', 'Browse': '/data/mathurs2/input', '--OUTDIR--': '/data/mathurs2/output', 'Browse0': '', '--ANNOTATION--': 'hg38_45'}
['/data/mathurs2/input/sample1-tumor.R2.fastq.gz', '/data/mathurs2/input/sample1-normal.R2.fastq.gz', '/data/mathurs2/input/sample1-normal.R1.fastq.gz', '/data/mathurs2/input/sample1-tumor.R1.fastq.gz']
Traceback (most recent call last):
  File "/vf/users/CCBR/projects/techDev/RENEE/main.py", line 16, in <module>
    sys.exit(main())
             ^^^^^^
  File "/vf/users/CCBR/projects/techDev/RENEE/src/renee/__main__.py", line 1479, in main
    args.func(args)
  File "/vf/users/CCBR/projects/techDev/RENEE/src/renee/gui.py", line 178, in launch_gui
    tmp_dir=get_tmp_dir(),
            ^^^^^^^^^^^^^
TypeError: get_tmp_dir() missing 2 required positional arguments: 'tmp_dir' and 'outdir'

Thanks for reporting this! I believe I've fixed it now

samarth8392 · 2024-08-05T22:16:31Z

Thanks for updating the two util functions. Now the error is coming from

RENEE/src/renee/run.py

Line 59 in 9e12660

if sub_args.wait:

and

RENEE/src/renee/gui.py

Line 185 in 9e12660

allout = run_in_context(run_args)

I think --wait must be added to the run_args ??

Traceback (most recent call last):
  File "/vf/users/CCBR/projects/techDev/RENEE/main.py", line 16, in <module>
    sys.exit(main())
             ^^^^^^
  File "/vf/users/CCBR/projects/techDev/RENEE/src/renee/__main__.py", line 1477, in main
    args.func(args)
  File "/vf/users/CCBR/projects/techDev/RENEE/src/renee/gui.py", line 185, in launch_gui
    allout = run_in_context(run_args)
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/vf/users/CCBR/projects/techDev/RENEE/src/renee/gui.py", line 243, in run_in_context
    run(args)
  File "/vf/users/CCBR/projects/techDev/RENEE/src/renee/run.py", line 59, in run
    if sub_args.wait:
       ^^^^^^^^^^^^^
AttributeError: 'Namespace' object has no attribute 'wait'

kelly-sovacool · 2024-08-05T22:16:55Z

ok, I'm working through a couple more errors after this one, will let you know when it's working on my end

kelly-sovacool · 2024-08-05T22:27:28Z

Yes I'm working on going through and adding all the sub_args. I thought those with defaults would be automatically populated, but I guess that only happens via the CLI.

also: remove unnecessary sys.exit(0) from dryrun. causes incorrect test failure with pytest

samarth8392 · 2024-08-06T13:27:00Z

Thanks Kelly for fixing those errors, now it's related to SIF cache dir.

Traceback (most recent call last):
  File "/vf/users/CCBR/projects/techDev/RENEE/main.py", line 16, in <module>
    sys.exit(main())
             ^^^^^^
  File "/vf/users/CCBR/projects/techDev/RENEE/src/renee/__main__.py", line 1477, in main
    args.func(args)
  File "/vf/users/CCBR/projects/techDev/RENEE/src/renee/gui.py", line 180, in launch_gui
    singularity_cache=os.environ["SINGULARITY_CACHEDIR"],
                      ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen os>", line 679, in __getitem__
KeyError: 'SINGULARITY_CACHEDIR'

same behavior as before, just moved this from orchestrate() to its own function for the GUI to use too

samarth8392 · 2024-08-06T14:25:42Z

Yeyy! The dryrun works now. I didn't submit a full run but I don't think there should be any issues there. If this is good enough, send me a 👍 and I will approve the PR and merge the branches.

kelly-sovacool · 2024-08-06T14:33:12Z

Yeyy! The dryrun works now. I didn't submit a full run but I don't think there should be any issues there. If this is good enough, send me a 👍 and I will approve the PR and merge the branches.

I think it is worthwhile to try submitting a full run just in case, since we don't have any unit tests for that section of the code

they are no longer in shared resources dir

previous commit only fixed it for one, but there are two

samarth8392 · 2024-08-06T20:58:02Z

I just submitted a full test run, all the rules worked file but jobby failed.

run_jobby_on_snakemake_log: command not found

I think this is a RENEE issue because the file is in resources folder so it should be os.path.join('resources', 'run_jobby_on_snakemake_log'

RENEE/workflow/Snakefile

Line 232 in 2c930d7

    
           jobby_cmd = "run_jobby_on_snakemake_log " + join('logfiles', 'snakemake.log') + " | tee " + join('logfiles', 'snakemake.log.jobby') + " | cut -f2,3,18 > " + join('logfiles', 'snakemake.log.jobby.short')

also for spooker

run_jobby_on_snakemake_log logfiles/snakemake.log | tee logfiles/snakemake.log.jobby | cut -f2,3,18 > logfiles/snakemake.log.jobby.short
/usr/bin/bash: run_jobby_on_snakemake_log: command not found
CalledProcessError in file /data/BandayLab/CABO-exome-seq-Andrea/mathurs2/ccbr1332/test/output/workflow/Snakefile, line 241:
Command 'set -euo pipefail;  run_jobby_on_snakemake_log logfiles/snakemake.log | tee logfiles/snakemake.log.jobby | cut -f2,3,18 > logfiles/snakemake.log.jobby.short' returned non-zero exit status 127.
  File "/data/BandayLab/CABO-exome-seq-Andrea/mathurs2/ccbr1332/test/output/workflow/Snakefile", line 241, in __onsuccess
OnSuccess
run_jobby_on_snakemake_log logfiles/snakemake.log | tee logfiles/snakemake.log.jobby | cut -f2,3,18 > logfiles/snakemake.log.jobby.short

kelly-sovacool · 2024-08-06T21:03:36Z

I just submitted a full test run, all the rules worked file but jobby failed.

run_jobby_on_snakemake_log: command not found

I think this is a RENEE issue because the file is in resources folder so it should be os.path.join('resources', 'run_jobby_on_snakemake_log'

run_jobby_on_snakemake_log logfiles/snakemake.log | tee logfiles/snakemake.log.jobby | cut -f2,3,18 > logfiles/snakemake.log.jobby.short
/usr/bin/bash: run_jobby_on_snakemake_log: command not found
CalledProcessError in file /data/BandayLab/CABO-exome-seq-Andrea/mathurs2/ccbr1332/test/output/workflow/Snakefile, line 241:
Command 'set -euo pipefail;  run_jobby_on_snakemake_log logfiles/snakemake.log | tee logfiles/snakemake.log.jobby | cut -f2,3,18 > logfiles/snakemake.log.jobby.short' returned non-zero exit status 127.
  File "/data/BandayLab/CABO-exome-seq-Andrea/mathurs2/ccbr1332/test/output/workflow/Snakefile", line 241, in __onsuccess
OnSuccess
run_jobby_on_snakemake_log logfiles/snakemake.log | tee logfiles/snakemake.log.jobby | cut -f2,3,18 > logfiles/snakemake.log.jobby.short

This is expected behavior if you did not run module load ccbrpipeliner before submitting the run.

We are no longer using the jobby file in the resources directory, instead we'd like to have "single source" of jobby (and spooker) which is added to the path when you load ccbrpipeliner.

samarth8392 · 2024-08-06T21:06:24Z

This is expected behavior if you did not run module load ccbrpipeliner before submitting the run.

We are no longer using the jobby file in the resources directory, instead we'd like to have "single source" of jobby (and spooker) which is added to the path when you load ccbrpipeliner.

Okay! Thanks!! In that case the test run was successful and I can now approve the PR. Great job @kelly-sovacool :)

kelly-sovacool · 2024-08-06T21:07:42Z

Go team!

kelly-sovacool added 5 commits January 15, 2024 21:13

feat: add gui code from ccbrpipeliner

185d9d1

copied from renee/main.py in ccbrpipeliner repo

refactor(gui): reorder imports, copy outside functions from ccbrpipel…

1593878

…iner/src

style: run pre-commit on renee

e5af71e

refactor(cli): add gui subcommand

681722f

ci: specify genome config

e7bc290

kelly-sovacool marked this pull request as draft January 16, 2024 03:03

kopardev added the RENEE RepoName label Jan 16, 2024

kelly-sovacool mentioned this pull request Feb 1, 2024

refactor xavier GUI CCBR/XAVIER#81

Closed

chore: Merge branch 'main' into refactor-renee-gui

e391f46

kelly-sovacool added 13 commits August 2, 2024 14:46

chore: Merge branch 'main' into refactor-renee-gui

6576721

refactor: rename gui.main() to gui.launch_gui()

b35eb6c

chore: add gui dependencies

2dbc89b

chore: add CCBR logo to resources

e78e72f

refactor: breakup main functions into separate files

a0116d4

to prevent circular imports and make the code easier to maintain

feat: make sure cli works without pip install

e4df1f1

source from CCBR/CHAMPAGNE#180

refactor: use relative imports everywhere

1b1446c

test: use ./bin/renee to invoke the CLI

d70f77a

rather than src/renee/__main__.py, so that the src files will be in the path via ./main.py

test: help pytest find src for python path

017cc30

fix: datetime usage

5b0d50e

feat: set default singularity sif dir

3d998a5

resolves #53

refactor: remove most global vars, set dynamically instead

0dd8131

docs: update CHANGELOG.md

2ab9433

kelly-sovacool changed the title ~~Refactor renee GUI~~ Refactor the renee python package and move the GUI here Aug 2, 2024

Merge branch 'main' into refactor-renee-gui

d91b173

kelly-sovacool marked this pull request as ready for review August 2, 2024 23:26

kelly-sovacool requested review from samarth8392 and kopardev August 2, 2024 23:26

kelly-sovacool self-assigned this Aug 2, 2024

kelly-sovacool added the ccbrpipeliner/7 label Aug 2, 2024

Merge branch 'main' into refactor-renee-gui

67a235d

kelly-sovacool and others added 3 commits August 5, 2024 13:46

Merge branch 'main' into refactor-renee-gui

d659060

chore: remove reference to champagne

f6f751b

fix(cli): make sure prog name is renee

8247801

fix: tmpdir needs default option + outdir

33b59f6

fix(gui): fix util function usage

9e12660

kelly-sovacool added 3 commits August 5, 2024 18:36

fix(gui): need all subargs in NameSpace, including defaults

68348af

refactor: move run_in_context() to run submodule

c9e9984

also: remove unnecessary sys.exit(0) from dryrun. causes incorrect test failure with pytest

refactor: default tmp dir and shared resources dir are None

36c52e0

refactor: write get_singularity_cache_dir()

adf6be9

same behavior as before, just moved this from orchestrate() to its own function for the GUI to use too

kelly-sovacool and others added 3 commits August 6, 2024 10:42

Merge branch 'main' into refactor-renee-gui

d0702cc

fix: do not change fqscreen path with shared resources

db74a4f

they are no longer in shared resources dir

fix: do not change EITHER fqscreen path with shared resources

65f07b6

previous commit only fixed it for one, but there are two

samarth8392 approved these changes Aug 6, 2024

View reviewed changes

samarth8392 merged commit 39938fd into main Aug 6, 2024
5 checks passed

samarth8392 deleted the refactor-renee-gui branch August 6, 2024 21:07

kelly-sovacool added this to the 2024-08 milestone Aug 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor the renee python package and move the GUI here #94

Refactor the renee python package and move the GUI here #94

kelly-sovacool commented Jan 16, 2024 •

edited

Loading

kelly-sovacool commented Aug 2, 2024 •

edited

Loading

kopardev commented Aug 5, 2024

samarth8392 commented Aug 5, 2024 •

edited

Loading

kelly-sovacool commented Aug 5, 2024

samarth8392 commented Aug 5, 2024

kelly-sovacool commented Aug 5, 2024

kelly-sovacool commented Aug 5, 2024

samarth8392 commented Aug 6, 2024

samarth8392 commented Aug 6, 2024

kelly-sovacool commented Aug 6, 2024

samarth8392 commented Aug 6, 2024 •

edited

Loading

kelly-sovacool commented Aug 6, 2024 •

edited

Loading

samarth8392 commented Aug 6, 2024

kelly-sovacool commented Aug 6, 2024

Refactor the renee python package and move the GUI here #94

Refactor the renee python package and move the GUI here #94

Conversation

kelly-sovacool commented Jan 16, 2024 • edited Loading

Changes

testing

Issues

PR Checklist

kelly-sovacool commented Aug 2, 2024 • edited Loading

kopardev commented Aug 5, 2024

samarth8392 commented Aug 5, 2024 • edited Loading

kelly-sovacool commented Aug 5, 2024

samarth8392 commented Aug 5, 2024

kelly-sovacool commented Aug 5, 2024

kelly-sovacool commented Aug 5, 2024

samarth8392 commented Aug 6, 2024

samarth8392 commented Aug 6, 2024

kelly-sovacool commented Aug 6, 2024

samarth8392 commented Aug 6, 2024 • edited Loading

kelly-sovacool commented Aug 6, 2024 • edited Loading

samarth8392 commented Aug 6, 2024

kelly-sovacool commented Aug 6, 2024

kelly-sovacool commented Jan 16, 2024 •

edited

Loading

kelly-sovacool commented Aug 2, 2024 •

edited

Loading

samarth8392 commented Aug 5, 2024 •

edited

Loading

samarth8392 commented Aug 6, 2024 •

edited

Loading

kelly-sovacool commented Aug 6, 2024 •

edited

Loading