Skip to content

Conversation

@mayasheth
Copy link
Contributor

No description provided.

Copy link

@kaybrand kaybrand left a comment

Choose a reason for hiding this comment

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

I like how you pulled the model directories out of the dataset directories. It is also nice that you supply a single merged CRISPRi training dataset to the train_model rule, using the CellType column to combine distinct datasets.

These are my suggestions from my first read-through:
In line 58 of the README, please correct 'saples' to 'samples'.
You refer several times to 'ct' and 'cd' in variable names. Renaming them to 'cell_type' and 'crispr_dataset' (or another name) would make your code more readable.
On line 19 of Snakefile_training, you set config["results_dir"] to be an absolute path. But if this were already an absolute path, the results dir might end up looking something like /oak/stanford/groups/engreitz/Users/kaybrand/ENCODE_rE2G/oak/stanford/groups/engreitz/Users/kaybrand/ENCODE_rE2G/results. I advise checking if the path starts from the root, checking if directory exists, and creating it if it does not, then saving the path to config["results_dir"].

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.

3 participants