Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-14593

Clarify use of repo subdirectory in ap_verify_hits2015 dataset

    Details

      Description

      The README of ap_verify_hits2015 currently says the following about the repo subdirectory: "Butler repo into which raw data can be ingested. This should be copied to an appropriate location before ingestion. Currently contains the appropriate DECam _mapper file."

      While this text does suggest a user should copying repo to a new location and then somehow make ingestion happen, it would benefit from clarification. Specifically:

      1) The README should make it clear that the repo directory should not be altered in-place, as edits to a local copy of ap_verify_hits2015 would prevent users from git-pulling any changes,

      and

      2) Documentation about ingestion for this specific dataset should be added/linked appropriately (presumably via ap_verify's ingest_dataset.py script).

        Attachments

          Activity

          Hide
          krzys Krzysztof Findeisen added a comment -

          Meredith Rawls, for #2, would a link to the ap_verify repository work for now? It's kind of hard to link from a readme to Sphinx docs, especially when the docs haven't been published yet.

          Show
          krzys Krzysztof Findeisen added a comment - Meredith Rawls , for #2, would a link to the ap_verify repository work for now? It's kind of hard to link from a readme to Sphinx docs, especially when the docs haven't been published yet.
          Hide
          mrawls Meredith Rawls added a comment -

          That's true. Still, I think it would be best to have some example commands users could run on the command line to turn this as-is "raw" dataset (straight from cloning) into an ingested one that is ready for whatever processing they would like to do. Something like (1) git clone with lfs enabled, (2) copy the repo directory to somewhere useful, (3) ref cats party, (4) ingestion one-liner via ap_verify assuming all goes well. It doesn't have to be detailed and cover every use case, it's just a starting point, similar to the obs_decam repo README. Certainly you can point users to the ap_verify repository for more details!

          Show
          mrawls Meredith Rawls added a comment - That's true. Still, I think it would be best to have some example commands users could run on the command line to turn this as-is "raw" dataset (straight from cloning) into an ingested one that is ready for whatever processing they would like to do. Something like (1) git clone with lfs enabled, (2) copy the repo directory to somewhere useful, (3) ref cats party, (4) ingestion one-liner via ap_verify assuming all goes well. It doesn't have to be detailed and cover every use case, it's just a starting point, similar to the obs_decam repo README. Certainly you can point users to the ap_verify repository for more details!
          Hide
          krzys Krzysztof Findeisen added a comment -

          Hi Eric Morganson, can you review this documentation fix and see if it makes the role of repo and ingestion clearer for you? You should have already gotten notifications for the corresponding GitHub pull requests; you can add line-by-line comments from the "Files changed" tabs.

          In addition to the requested changes to ap_verify_hits2015, I've made similar wording changes in ap_verify_testdata and ap_verify_dataset_template, and fixed a mistake in the ap_verify docs that I discovered while copying and pasting the command lines.

          Show
          krzys Krzysztof Findeisen added a comment - Hi Eric Morganson , can you review this documentation fix and see if it makes the role of repo and ingestion clearer for you? You should have already gotten notifications for the corresponding GitHub pull requests; you can add line-by-line comments from the "Files changed" tabs. In addition to the requested changes to ap_verify_hits2015 , I've made similar wording changes in ap_verify_testdata and ap_verify_dataset_template , and fixed a mistake in the ap_verify docs that I discovered while copying and pasting the command lines.
          Hide
          emorganson Eric Morganson added a comment -

          Certainly all the point 1) seems covered.

          Assuming that ingest_dataset.py works. Then 2) is covered.

          Show
          emorganson Eric Morganson added a comment - Certainly all the point 1) seems covered. Assuming that ingest_dataset.py works. Then 2) is covered.
          Hide
          krzys Krzysztof Findeisen added a comment -

          Merged. Thank you both Eric Morganson and Meredith Rawls for your reviews!

          Show
          krzys Krzysztof Findeisen added a comment - Merged. Thank you both Eric Morganson and Meredith Rawls for your reviews!

            People

            • Assignee:
              krzys Krzysztof Findeisen
              Reporter:
              mrawls Meredith Rawls
              Reviewers:
              Eric Morganson
              Watchers:
              Eric Morganson, Krzysztof Findeisen, Meredith Rawls
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel