timcornwell
commented
7 years ago On 24 Oct 2017, at 7:11 pm, Piers Harding notifications@github.com wrote:
Hi Tim -
This pull request contains all of the suggestions and notes made throughout the review of https://jira.ska-sdp.org/browse/TSK-1823 https://jira.ska-sdp.org/browse/TSK-1823 . You can either accept the pull request (which will be automatically merged into the master branch) or open dialogue and commentary on the contents of the commits.
As an alternative, you can add my repo as an alternative remote, pull the TSK-1823 branch and cherry-pick specific commits onto your copy of master (that you would then push to remote).
I decided to do the latter. I’ve been looking at the commits in detail. The only one that I don’t want to accept in total is the change of kwargs. Your proposed change to image_gather_scatter introduced a regression that wasn’t caught by the test cases.
Other than that, I think I’ll be able to take all of your suggestions. I’m currently working in a branch which I will commit back to the master when I’m convinced that everything is ok.
Cheers, Tim
Cheers, Piers Harding.
You can view, comment on, or merge this pull request online at:
https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5 https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5 Commit Summary
TSK-1823: expand docs and script testing TSK-1823: convert_blockvisibility_to_visibility TSK-1823: PEP-0008 TSK-1823: silence lint checkers TSK-1823: fix NotImplemented to NotImplementedError TSK-1823: fix ambiguous log TSK-1823: remove redundant redeclaration TSK-1823: suppress tests under Jenkins/Travis TSK-1823: fix redeclartions and lint check TSK-1823: add notebook launcher to make TSK-1823: lint checking arl/image TSK-1823: lint checking arl/imaging TSK-1823: lint checking arl/calibration TSK-1823: lint checking arl/fourier_transforms TSK-1823: lint checking arl/graphs TSK-1823: lint checking arl/imaging TSK-1823: lint checking arl/pipelines TSK-1823: lint checking arl/skycomponent TSK-1823: lint checking arl/util TSK-1823: lint checking arl/visibility TSK-1823: lint checking arl/ TSK-1823: lint checking tests/ TSK-1823: lint checking TSK-1823: fix lint checks TSK-1823: remove kwargs references TSK-1823: add example Docker container support TSK-1823: add Docker documentation TSK-1823: rejig Docker build TSK-1823: add Docker Swarm instructions TSK-1823: Dask workers require arl/data TSK-1823: a Jupyter notebook extensions TSK-1823: Deploy Dask cluster using Docker TSK-1823: swap type() for isinstance() File Changes
A .dockerignore https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-0 (57) M .gitignore https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-1 (4) A Dockerfile https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-2 (81) A Makefile https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-3 (119) M README.md https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-4 (254) M arl/init.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-5 (4) M arl/calibration/calibration.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-6 (20) M arl/calibration/operations.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-7 (10) M arl/calibration/peeling.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-8 (7) M arl/calibration/solvers.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-9 (34) M arl/data/data_models.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-10 (248) M arl/data/parameters.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-11 (18) M arl/data/persist.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-12 (2) M arl/data/polarisation.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-13 (69) M arl/fourier_transforms/init.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-14 (2) M arl/fourier_transforms/convolutional_gridding.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-15 (33) M arl/fourier_transforms/fft_support.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-16 (5) M arl/graphs/dask_init.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-17 (14) M arl/graphs/dask_minimal.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-18 (2) M arl/graphs/generic_graphs.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-19 (9) M arl/graphs/graphs.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-20 (20) M arl/graphs/vis.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-21 (8) M arl/image/cleaners.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-22 (160) M arl/image/deconvolution.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-23 (20) M arl/image/gather_scatter.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-24 (14) M arl/image/iterators.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-25 (17) M arl/image/operations.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-26 (108) M arl/image/solvers.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-27 (1) M arl/imaging/base.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-28 (37) M arl/imaging/facets_wprojection.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-29 (12) M arl/imaging/facets_wstack.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-30 (10) M arl/imaging/iterated.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-31 (5) M arl/imaging/params.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-32 (47) M arl/imaging/timeslice.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-33 (29) M arl/imaging/weighting.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-34 (21) M arl/imaging/wprojection.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-35 (2) M arl/imaging/wprojection_wstack.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-36 (3) M arl/imaging/wstack.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-37 (19) M arl/pipelines/distributed_setup.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-38 (2) M arl/pipelines/graphs.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-39 (1) M arl/pipelines/support.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-40 (2) M arl/skycomponent/operations.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-41 (99) M arl/util/coordinate_support.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-42 (2) M arl/util/graph_support.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-43 (76) M arl/util/primary_beams.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-44 (7) M arl/util/run_unittests.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-45 (1) M arl/util/testing_support.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-46 (51) M arl/util/timing.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-47 (3) M arl/visibility/init.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-48 (2) M arl/visibility/base.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-49 (82) M arl/visibility/coalesce.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-50 (122) M arl/visibility/gather_scatter.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-51 (10) M arl/visibility/iterators.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-52 (6) M arl/visibility/operations.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-53 (8) A docker/boot.sh https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-54 (5) M examples/performance/pipelines-timings-analyse.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-55 (3) M requirements.txt https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-56 (1) M tests/test_array_functions.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-57 (23) M tests/test_calibration_operations.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-58 (28) M tests/test_calibration_peeling.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-59 (10) M tests/test_calibration_solvers.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-60 (37) M tests/test_convolutional_gridding.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-61 (21) M tests/test_fft_support.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-62 (4) M tests/test_generic_graph.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-63 (27) M tests/test_graph_support.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-64 (29) M tests/test_graphs.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-65 (55) M tests/test_image_deconvolution.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-66 (19) M tests/test_image_msclean.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-67 (4) M tests/test_image_operations.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-68 (50) M tests/test_image_solvers.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-69 (20) M tests/test_imaging.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-70 (21) M tests/test_imaging_params.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-71 (17) M tests/test_parameters.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-72 (5) M tests/test_pipelines.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-73 (14) M tests/test_polarisation.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-74 (27) M tests/test_quality_assessment.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-75 (6) M tests/test_skycomponent.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-76 (8) M tests/test_visibility_coalesce.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-77 (5) M tests/test_visibility_gather_scatter.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-78 (16) M tests/test_visibility_operations.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-79 (36) M tests/test_weighting.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-80 (17) A tools/README.md https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-81 (17) A tools/ansible.cfg https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-82 (243) A tools/docker.yml https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-83 (73) A tools/inventory/docker https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-84 (13) A tools/inventory/group_vars/all https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-85 (4) A tools/ssh.config https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-86 (39) M util/read_oskar_vis.py https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5/files#diff-87 (5) Patch Links:
https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5.patch https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5.patch https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5.diff https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/SKA-ScienceDataProcessor/algorithm-reference-library/pull/5, or mute the thread https://github.com/notifications/unsubscribe-auth/ADv3XY05ATWUb79WihTrktWLlZqqEoQ6ks5svihHgaJpZM4QE3I2.
piersharding
Hi Tim -
This pull request contains all of the suggestions and notes made throughout the review of https://jira.ska-sdp.org/browse/TSK-1823 . You can either accept the pull request (which will be automatically merged into the master branch) or open dialogue and commentary on the contents of the commits.
As an alternative, you can add my repo as an alternative remote, pull the TSK-1823 branch and cherry-pick specific commits onto your copy of master (that you would then push to remote).
Cheers, Piers Harding.