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

Cleanup utils tests to after pytest conversion

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: utils

      Description

      Although most of the tests in the utils package have been updated for py.test, there are a few cleanups left to do in it.

        Attachments

          Issue Links

            Activity

            Hide
            Parejkoj John Parejko added a comment -

            Oog. Considering our conversation (and Ian Sullivan's comments) I'm not sure. I suppose Tim Jenness is the arbiter of all things "coding standard".

            Show
            Parejkoj John Parejko added a comment - Oog. Considering our conversation (and Ian Sullivan 's comments) I'm not sure. I suppose Tim Jenness is the arbiter of all things "coding standard".
            Hide
            tjenness Tim Jenness added a comment -

            I have no idea the reasoning behind the special exemption for one-line docstrings. Why does the style guide have an opinion on this? I wonder if Kian-Tat Lim can remember the background here. Did doxygen require it? Why deviate from the PEP? This would seem to be a candidate for a quick revisit once Jonathan Sick finishes this round of style guide updates.

            Show
            tjenness Tim Jenness added a comment - I have no idea the reasoning behind the special exemption for one-line docstrings. Why does the style guide have an opinion on this? I wonder if Kian-Tat Lim can remember the background here. Did doxygen require it? Why deviate from the PEP? This would seem to be a candidate for a quick revisit once Jonathan Sick finishes this round of style guide updates.
            Hide
            mtpatter Maria Patterson [X] (Inactive) added a comment -

            Ok, not sure about the one-line docstrings, but looking through everything else, the shebang is taken out of testGetTempFilePath.py in this PR but left in the others. Also the license bit at the top is in some files but not others, and where it is left, there are 3 variations of the copyright years line. Not sure if you want to make things consistent here in this PR, but just pointing it out. Otherwise everything else seems ok.

            Show
            mtpatter Maria Patterson [X] (Inactive) added a comment - Ok, not sure about the one-line docstrings, but looking through everything else, the shebang is taken out of testGetTempFilePath.py in this PR but left in the others. Also the license bit at the top is in some files but not others, and where it is left, there are 3 variations of the copyright years line. Not sure if you want to make things consistent here in this PR, but just pointing it out. Otherwise everything else seems ok.
            Hide
            Parejkoj John Parejko added a comment -

            Thank you for the review. I've updated based on your comments and pushed.

            The one remaining question is whether I should go with your recommendation to call setup_module from if _main_? I've pushed a change with that in place for folks to examine. Comments welcome.

            Show
            Parejkoj John Parejko added a comment - Thank you for the review. I've updated based on your comments and pushed. The one remaining question is whether I should go with your recommendation to call setup_module from if _ main _ ? I've pushed a change with that in place for folks to examine. Comments welcome.
            Hide
            Parejkoj John Parejko added a comment -

            Done and Merged. Thanks for the comments, all.

            Show
            Parejkoj John Parejko added a comment - Done and Merged. Thanks for the comments, all.

              People

              • Assignee:
                Parejkoj John Parejko
                Reporter:
                Parejkoj John Parejko
                Reviewers:
                Maria Patterson [X] (Inactive)
                Watchers:
                John Parejko, Maria Patterson [X] (Inactive), Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel