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

add run_batch script formatted for CPG #179

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

ErinWeisbart
Copy link
Member

Adds a run_batch script with formatting that matches data organization in the Cell Painting Gallery.
Uses run_batch_general.py as template with updating, cleanup, and changed file structure

@bethac07 is there any reason that I shouldn't perform similar cleanup on run_batch_general.py:

  • f-string formatting for legibility
  • only using posix.path instead of a combo of posix.path and os.path
  • (I will not deprecate batch jobs in run_batch_general.py)

@bethac07
Copy link
Collaborator

I don't see any reason not to clean up run_batch_general, but if the major distinction is just path structure, any major reason to not to just have one script that takes a parameter for CPG? (Ie have the path structures in a dictionary with keys for standard, cpg, etc.

Tbh, I've been longing for ages to make this a CLI with flags rather than the commenting and uncommenting nonsense, if you agree that it's reasonable to do one file here, this would be a good excuse to do that work too. What do you think? Could do it together next week?

@bethac07
Copy link
Collaborator

(or do it yourself, of course, just if it makes it more fun)

@ErinWeisbart
Copy link
Member Author

No reason not to combine them. I made this version for a project I was working on and then thought it would be nice to polish and add to the repo, but the extra work to combine (and make CLI) seems worth it!

@ErinWeisbart ErinWeisbart marked this pull request as ready for review October 8, 2024 20:56
Copy link
Collaborator

@bethac07 bethac07 left a comment

Choose a reason for hiding this comment

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

Make sure the parser defaults match the function defaults, otherwise, very nice work!

parser.add_argument(
"--plate-format",
dest="plate_format",
default=384,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Make sure that here and elsewhere (ie rows and columns), the default here matches the function default. Otherwise, if the user DOESN'T pass --plate-format, it will default to 384, so even if they passed rows, it would overwrite them with the 384 plate format.

help="Name of the pipeline to overwrite defaults of Zproj.cppipe, illum.cppipe, qc.cppipe, assaydev.cppipe, analysis.cppipe.",
)
parser.add_argument(
"--outputstructure",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor preference for this to be output-structure, but I don't feel strongly about it if you'd rather not

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