search

ScottishCovidResponse / data_pipeline_api

API to access the data pipeline
BSD 3-Clause "New" or "Revised" License
0 stars 2 forks source link

Distributions returned from api to use specified random generator #96

Closed vinopm closed 4 years ago

vinopm commented 4 years ago

All distributions returned from api to use specified random generator + some clean up

vinopm commented 4 years ago

@johnnonweiler

codecov-commenter commented 4 years ago

Codecov Report

Merging #96 into master will decrease coverage by 1.51%. The diff coverage is 83.48%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #96      +/-   ##
============================================
- Coverage     78.27%   76.76%   -1.52%     
- Complexity        0      181     +181     
============================================
  Files            17       64      +47     
  Lines          1404     2208     +804     
  Branches          0       38      +38     
============================================
+ Hits           1099     1695     +596     
- Misses          305      481     +176     
- Partials          0       32      +32
Impacted Files Coverage Δ Complexity Δ
...i/src/main/java/uk/ramp/metadata/MetadataItem.java 70.93% <50.00%> (ø) 36.00 <3.00> (?)
...c/main/java/uk/ramp/distribution/Distribution.java 79.66% <66.66%> (ø) 17.00 <0.00> (?)
.../java/uk/ramp/parameters/ComponentsSerializer.java 78.26% <70.00%> (ø) 5.00 <2.00> (?)
...ava/uk/ramp/parameters/ComponentsDeserializer.java 84.00% <71.42%> (ø) 4.00 <1.00> (?)
...src/main/java/uk/ramp/toml/TOMLInputDecorator.java 53.84% <80.00%> (ø) 2.00 <2.00> (?)
...rc/main/java/uk/ramp/toml/TOMLOutputDecorator.java 54.54% <80.00%> (ø) 2.00 <2.00> (?)
...c/main/java/uk/ramp/mapper/DataPipelineMapper.java 88.00% <88.00%> (ø) 3.00 <3.00> (?)
...ard_api/src/main/java/uk/ramp/api/StandardApi.java 89.65% <90.47%> (ø) 9.00 <3.00> (?)
.../file_api/src/main/java/uk/ramp/config/Config.java 100.00% <100.00%> (ø) 4.00 <1.00> (?)
...api/src/main/java/uk/ramp/yaml/BaseYamlWriter.java 84.61% <100.00%> (ø) 2.00 <0.00> (?)
... and 64 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1ce5d0e...e32a0e4. Read the comment docs.

vinopm commented 4 years ago

@mrow84 Thanks for your feedback. The data registry download script seems to expect the config.yaml file to have data_directory within run_metadata? It fails with an error when data_directory is specified at the top level and when run_metadata does not contain data_directory

mrow84 commented 4 years ago

Ah yeah, you're right, the download script does take it from the run_metadata. Does the java implementation of the file API now just use the data_directory in the run_metadata section internally? If so I will change the python implementation accordingly and update the spec. I wonder whether we should move the other parameters (e.g. run_id) inside there as well? The output run_id might be different to the input one, but I think that is fine.

vinopm commented 4 years ago

@mrow84 Yeah, i just changed the java api in this PR to pick up the data_directory from within run_metadata. The other params such as run_id are still at the root level. We could move it, not sure what would make most sense from the API perspective

mrow84 commented 4 years ago

Ok well in the first instance I will move the data_directory in the same way you have here. We can perhaps defer the rest to a later time.

vinopm commented 4 years ago

Agreed, thanks @mrow84