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

Refactor extract_tiles_wrapper #331

Closed
bikegeek opened this issue Dec 2, 2019 · 2 comments
Closed

Refactor extract_tiles_wrapper #331

bikegeek opened this issue Dec 2, 2019 · 2 comments
Assignees

Comments

@bikegeek
Copy link
Contributor

bikegeek commented Dec 2, 2019

The extract tiles wrapper needs to be refactored so that 1) there are more methods than the run_at_time() method and 2) to not rely on a process id to differentiate one extract tile wrapper run from another. Issue #2 prevents the ability to use pre-existing data for unit testing. Currently, the only testing that can be performed is system (end-to-end) testing where the extract tiles wrapper must be run in order to check final output.

@bikegeek bikegeek added this to the METplus Future Versions milestone Dec 2, 2019
@bikegeek bikegeek self-assigned this Dec 5, 2019
bikegeek added a commit that referenced this issue Dec 5, 2019
…rectory that is the current process ID and remove existing tmp file before appending to it.

 This will prevent appending identical information (and headers) to the tmp file for a given storm, when re-running extract tiles with existing tc pairs output.  This will result in an error as it is assumed that the tmp file will only have one header.
@bikegeek
Copy link
Contributor Author

bikegeek commented Dec 5, 2019

removed the creation of the tmp/ subdirectory in extract_tiles_wrapper.py. Also checking for pre-existing tmp file (before appending header and storm info) in feature_util.py (retrieve_and_regrid()). This check is needed to support the re-running of extract tiles wrapper, or when running the extract tiles wrapper using pre-existing tc pairs output.

bikegeek added a commit that referenced this issue Dec 6, 2019
…s id

1) this will allow extract tiles to be run using the tc-pairs data from previous runs
2) check for the existence of a tmp file in the tmp directory that matches the storm
   of interest and delete it if it does.  If you fail to do this, subsequent runs of
   extract tiles wrapper will append identical information (headers included) which
   result in errors later on when we are parsing these tmp files for information.
bikegeek added a commit that referenced this issue Dec 6, 2019
Break up the run_at_time() method into smaller methods so we can do
proper unit testing.
bikegeek added a commit that referenced this issue Dec 6, 2019
…nger depends on process id.

Merge branch 'feature_331_refactor_extract_tiles' into develop
@bikegeek
Copy link
Contributor Author

bikegeek commented Dec 6, 2019

extract tiles wrapper is now refactored so that it is unit-testable by breaking up the run_at_time() method to invoke other methods:
-tc_files_exist()
-do_filtering(cur_init, filter_name)
-create_results_files(sorted_storm_ids, cur_init, filter_name, tmp_dir)
-cleanup(tmp_dir)

Now the wrapper is unit-testable (refer to Github Issue #334 refactor extract tiles wrapper tests) and can be run/re-run using tc-pairs data from a different process.

@bikegeek bikegeek closed this as completed Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant