Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cellfinder workflow CLI input argument with tests #23

Merged
merged 74 commits into from
Oct 13, 2023

Conversation

sfmig
Copy link
Contributor

@sfmig sfmig commented Oct 5, 2023

A script to simulate a typical cellfinder workflow. It builds up on previous work from PRs #15 and #20 (none of which we merged).

We wanted

  • to define workflows that can fetch data from GIN and also locally - note these workflows will take the required input parameters from an input config file; and
  • to allow the input config to be set to some default values, but also allow them to be customised

The current solution takes the path to a configuration json file as a command line input.

If no input json file is passed as a CLI argument, the default configuration defined at brainglobe_workflows/cellfinder/default_config.json is used. This default configuration fetches data from a GIN repo, creates a .cellfinder_workflows cache subdirectory at the location the script is launched from, and saves to it the downloaded data and the output results.

Example usage

  • to pass a custom configuration, run from the cellfinder_main.py parent directory:
    python cellfinder_main.py --config path/to/input/config.json
  • to use the default configuration, run:
    python cellfinder_main.py

It ended up being a larger PR than I thought, apologies! But hopefully it crystallises the structure we discussed these past few days (see #9)

@codecov
Copy link

codecov bot commented Oct 5, 2023

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (8665e74) 0.00% compared to head (f16c6b9) 92.56%.
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##            main      #23       +/-   ##
==========================================
+ Coverage   0.00%   92.56%   +92.56%     
==========================================
  Files          1        2        +1     
  Lines          5      121      +116     
==========================================
+ Hits           0      112      +112     
- Misses         5        9        +4     
Files Coverage Δ
brainglobe_workflows/__init__.py 60.00% <60.00%> (ø)
brainglobe_workflows/cellfinder/cellfinder_main.py 93.96% <93.96%> (ø)

... and 1 file with indirect coverage changes

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

This was referenced Oct 9, 2023
@sfmig sfmig marked this pull request as ready for review October 9, 2023 16:07
@sfmig sfmig requested a review from alessandrofelder October 9, 2023 16:54
Copy link
Member

@alessandrofelder alessandrofelder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @sfmig
I think the source code looks good - made some minor comments!

As discussed, we may want to refactor the tests a bit in a separate PR.

MANIFEST.in Outdated Show resolved Hide resolved
brainglobe_workflows/cellfinder/cellfinder_main.py Outdated Show resolved Hide resolved
brainglobe_workflows/cellfinder/cellfinder_main.py Outdated Show resolved Hide resolved
brainglobe_workflows/cellfinder/cellfinder_main.py Outdated Show resolved Hide resolved
@sfmig sfmig mentioned this pull request Oct 13, 2023
7 tasks
@sfmig sfmig merged commit 86754ed into main Oct 13, 2023
11 checks 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