search

AllenInstitute / ophys_etl_pipelines

Pipelines and modules for processing optical physiology data
Other
9 stars 5 forks source link

Validate inputs with argschema #596

Closed mikejhuang closed 11 months ago

mikejhuang commented 1 year ago
aamster commented 1 year ago

@mikejhuang I need to see that the tests are passing. Linting errors are blocking the tests from running. Please fix the linting errors so I can see that the tests are passing.

mikejhuang commented 1 year ago

@mikejhuang I need to see that the tests are passing. Linting errors are blocking the tests from running. Please fix the linting errors so I can see that the tests are passing.

Are you sure? Won't this make it difficult for you to read the diff? Here are the workflow tests passing with an on prem machine (Synapse)

image

mikejhuang commented 1 year ago

Alternatively, there might be a way to find all classes that end with Module and call .inputs for each class as a smoke test. That way the schema will be automatically checked for each module.

Presently there aren't any tests for

denoising

  1. _denoising
  2. denoising_finetuning
  3. denoising_training

roi_classification

  1. create_test_train_split
  2. generate_correlation_projection
  3. generate_thumbnails

trace_processing

  1. demix_traces
  2. dff_calculation
  3. neuropil_correction
  4. trace_extraction

Given that this was supposed to be a 3 point ticket (notwithstanding the extra work for the other ticket that came out of this), I didn't think creating the unit tests would be in the scope of this ticket. It would be caught in the integration tests that we decided to delegate much of the testing to.

aamster commented 1 year ago

@mikejhuang what you outlined above was supposed to be the purpose of this ticket. If there are things that can be tested then they should be tested rather than leaving it up to manual testing. If you need to increase the point count to work on it then please do it