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

Codify the expectation that in-place experiment construction does not rely on TileFetcher data #1389

Merged
merged 1 commit into from
Jun 19, 2019

Conversation

ttung
Copy link
Collaborator

@ttung ttung commented Jun 3, 2019

When we construct an experiment in-place, the source data comes from the tiles already on disk. Therefore, whatever image data we provide the TileFetcher that we pass into write_experiment_json should be discarded. This verifies that the data from the tiles on the disk are what gets constituted into the experiment, and that the data returned by TileFetcher is ignored.

Test plan: passed the test.
Fixes #1388

@ttung ttung requested a review from shanaxel42 June 3, 2019 22:54
@ttung ttung force-pushed the tonytung-inplace branch from 84c0b55 to fd944da Compare June 3, 2019 22:55
@codecov-io
Copy link

codecov-io commented Jun 3, 2019

Codecov Report

Merging #1389 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1389   +/-   ##
=======================================
  Coverage   89.45%   89.45%           
=======================================
  Files         152      152           
  Lines        5527     5527           
=======================================
  Hits         4944     4944           
  Misses        583      583

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 803d930...14cb0de. Read the comment docs.

@ttung ttung force-pushed the tonytung-inplace branch from fd944da to 286fd0d Compare June 6, 2019 22:47
… rely on TileFetcher data

When we construct an experiment in-place, the source data comes from the tiles already on disk.  Therefore, whatever image data we provide the TileFetcher that we pass into `write_experiment_json` should be discarded.  This verifies that the data from the tiles on the disk are what gets constituted into the experiment, and that the data returned by TileFetcher is ignored.

Test plan: passed the test.
Fixes #1388
@ttung ttung force-pushed the tonytung-inplace branch from 286fd0d to 14cb0de Compare June 12, 2019 18:50
@ttung ttung merged commit 4b8c49e into master Jun 19, 2019
@ttung ttung deleted the tonytung-inplace branch June 19, 2019 07:34
ttung pushed a commit that referenced this pull request Jul 1, 2019
… to implement

#1389 codifies the expectation that in-place tile construction does not rely on the data from `tile_data()`.  This removes the requirement that in-place tile construction even needs to implement `tile_data()` by providing a default implementation.

Note that this doesn't preclude someone from implementing `tile_data()` (example: the structured data formatter in #1421) for cases where the TileFetcher can be used inplace or non-inplace.

Test plan: travis
ttung pushed a commit that referenced this pull request Jul 3, 2019
… to implement (#1429)

#1389 codifies the expectation that in-place tile construction does not rely on the data from `tile_data()`.  This removes the requirement that in-place tile construction even needs to implement `tile_data()` by providing a default implementation.

Note that this doesn't preclude someone from implementing `tile_data()` (example: the structured data formatter in #1421) for cases where the TileFetcher can be used inplace or non-inplace.

Test plan: travis
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.

Inplace conversion to Starfish format seems slow
3 participants