v0.1.0a1

[khoroshevskyi]

Probably bedboss is not ready yet, but I would like to here your comments

1) Composed all scripts together 2) Added some corrections to the scripts

Things to finish:

• [x] Add documentation
• [x] Add tests???
• [x] Add GitHub actions
• [x] #1

[nsheff]

This is really great! I think this is a great start and I think this is way better structure than what we had before. I wasn't able to review everything because I ran into some blockers that I couldn't get past, though. There's a lot we can do to clean this up and make it better. Here's my suggestions for you:

Docs feedback:

• [x] please construct a proper mkdocs setup for this. All it requires is copying and editing a mkdocs yaml file and then I will be able to view the docs in a nice, linked way with mkdocs serve. You already have the structure set up correctly.
• [x] please in general follow the documentation pattern using mkdocs that we use for all our other lab packages.
• [x] why is usage printed both in docs/README.md and docs/usage.md?
• [x] usage docs page is outdated
  • to see how to auto-update a usage docs file, see: https://github.com/pepkit/looper/blob/master/update-usage-docs.sh
• [x] bedboss should use VersionInHelpParser
  • https://github.com/pepkit/ubiquerg/blob/1d79b7c2d2e2319b3221b30cf63ccb377ac1fbc4/ubiquerg/cli_tools.py#L17-L28
  • Example of how to use it: https://github.com/pepkit/divvy/blob/ef51a9e5225b7ce1b8dd36713442687816298422/divvy/compute.py#L429
• [x] I think instead of bedboss boss it should be bedboss all. Or should we introduce subparsers, bedboss run qc bedboss run make bedboss run all ?
• [x] Can you write a simple example test tutorial, or transfer the other bedbase pipeline tutorial, provide some dummy test example data, to show me how to actually run this?
• [x] BedMaker class description is in the init docstring instead of in the class docstring. Please check your docstrings for content and format. You may be able to find a linter to help with the format part.
• [x] I found no documentation on how to write a bedbase config file.

Code feedback:

• [x] The way this argumentParser works strikes me as strange (using args = parser.parse_args(sys.argv[1:2]) and creating separate argument parsers. This could become a maintenance challenge. I believe the accepted way to build an ArgumentParser with multiple commands like this is to use subparsers. This is what we have done in the past. See for example: https://github.com/pepkit/looper/blob/df535be2a0bd7257ee37148cb601d688ec557909/looper/__init__.py#L61
• [x] config_db_local.yaml is duplicated in pipeline_schemas/ and bedboss/
• [x] please come up with a strategy for testing this. At least something; discuss with me if needed. I suggest just creating a simple smoketest to start: create a very small BED file, and have the test run it through the whole pipeline, maybe even in python. Confirm some of the outputs. That's at least something to show how it runs.
• [x] double-check your docstrings on your exceptions, some are wrong/generic/confusing.
• [x] What is "OSMException" ? I don't know what OSM means.
• [x] I was not able to find a pipeline_interface -- is the intent to run this with looper? There is one that says pipeline_name BEDMAKER in the pipeline_schemas folder, but this is confusing to me... Actually I can't really follow what the items in pipeline_schemas are for.
• [x] Converting from string.format() syntax to using Python f-strings throughout. They are much easier to read and understand.
• [ ] bedboss qc (@xuebingjie1990 ):
  • [ ] CLI appears to not provide pypiper/divvy/logmuse args, but other pipelines do.
  • [x] bedboss qcused theest_line.sh` shell script under the 'bedmaker' folder, which confused me.
  • [ ] bedqc.py doesn't make sense to me. It has several problems:
    1. It writes a file output instead of using pipestat to report a result.
    2. it only records a result if there's a problem.
    3. It records a result as prose instead of as machine-understanble numbers.
    4. It just discards the results if they pass the cutoff. Instead, this should be reporting a result of:
       • number of regions
       • file size
       • mean region width ... these are each independent reportable results, whether the file passes or fails. they should be recorded in the stats/output of the pipeline.
  • [x] it's not clear why bedboss qc uses --bedfile and then bedboss make uses --input-file -- can we standardize these inputs somehow? --- OK: it uses bedfile, because bedqc can take only bed files
• [x] bedboss make(@xuebingjie1990 ): bedmaker could be improved a lot.
  • [x] get_bed_type is doing 2 tasks, these should be split into functions with appropriate names.
  • [x] need more info on 'standardizing chromosome names (docstring, etc)'
  • [x] it's not clear what bedtype is
  • [x] docstring to _get_chrom_sizes doesn't tell the whole story. this function is also potentially creating something. It should be divided into two appropriately named functions.
  • [x] Several bedmaker arguments makes no sense. Please go through all bedmaker CLI args, think about them carefully, and make them understandable. For example:
    1. for -n the docs seem to indicate a boolean but then shouldn't it be a flag argument? What's the variable for?: "-n NARROWPEAK, --narrowpeak NARROWPEAK whether the regions are narrow (transcription factor implies narrow, histone mark implies broad peaks)"
    2. for -t, the docs say it's a bigwig file, but the arg is called INPUT_TYPE, and it's not clear what goes there? "-t INPUT_TYPE, --input-type INPUT_TYPE a bigwig or a bedgraph file that will be converted into BED format"
• [x] bedboss stat
  • the argparser is incorrect; it should specify which are the required fields but it doesn't. It doesn't lit config file as a required argument, but if you don't provide it, it fails.
  • I was unable to proceed further because I found no documentation on how to write a bedbase config file.

[khoroshevskyi]

@nsheff @xuebingjie1990 It seams that bedboss is ready for it's first release:) Could you please check it again, and see if something else should be added