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

Minor bugs in template for roundtable_aiohttp_bot

    XMLWordPrintable

    Details

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

      Description

      I just ran the template package (fresh git pull of master) to make a roundtable_aiohttp_bot app and ran into a few small issues:

      • src/<name>/__init__.py adds "metadata" to __all__ but that symbol does not exist.
      • There is an if statement based on Python version, but the two cases are identical (not a bug, but please consider simplifying the file to remove this).
      • src/<name>/handlers/__init__.py adds "init_routes" to __all__ but that symbol does not exist.

      I also see these issues in segwarides yet it runs, so maybe the two things I think are bugs are not. If they actually work then a brief comment explaining what is going on in the generated files would be appreciated. Certainly they make flake8 very unhappy.

      Finally: the newly created files could not be committed because isort said some imports were out of order.
      I used the package name "owl" in case that makes a difference.

        Attachments

          Activity

          Hide
          jsick Jonathan Sick added a comment -

          Russell Owen, I'll make the fixes, in the meantime:

          For #1, feel free to remove that metadata; that was missed from previous changes to the template.

          For #2, they are different lines, but if you're only developing on Python 3.8 you don't need the backport package ( importlib_metadata ).

          For #3, again feel free to remove the init_routes from }}{{_all_.

          Show
          jsick Jonathan Sick added a comment - Russell Owen , I'll make the fixes, in the meantime: For #1, feel free to remove that metadata ; that was missed from previous changes to the template. For #2, they are different lines, but if you're only developing on Python 3.8 you don't need the backport package ( importlib_metadata  ). For #3, again feel free to remove the init_routes from }}{{_ all _ .
          Hide
          rowen Russell Owen added a comment - - edited

          Another thing: are the __init__.py files the tests directory needed? I see them at every level (3 levels, 4 files). I'm guessing they are there so tests can find the handlers. But I thought such files aren't needed in Python 3.

          Show
          rowen Russell Owen added a comment - - edited Another thing: are the __init__.py files the tests directory needed? I see them at every level (3 levels, 4 files). I'm guessing they are there so tests can find the handlers. But I thought such files aren't needed in Python 3.
          Hide
          jsick Jonathan Sick added a comment - - edited

          That decision was based on feedback from Russ Allbery, who pointed out that it can be beneficial to keep test helpers in the tests "package" rather than adding them to the application or relying on fixtures from conftest.py exclusively. If you don't want this feature, feel free to delete those _init_.py files. Your tests will work without them. I don't think be taking them out of the template, though.

          Show
          jsick Jonathan Sick added a comment - - edited That decision was based on feedback from Russ Allbery , who pointed out that it can be beneficial to keep test helpers in the tests "package" rather than adding them to the application or relying on fixtures from conftest.py exclusively. If you don't want this feature, feel free to delete those _ init _.py files. Your tests will work without them. I don't think be taking them out of the template, though.
          Hide
          jsick Jonathan Sick added a comment -

          Finally: the newly created files could not be committed because isort said some imports were out of order.
          I used the package name "owl" in case that makes a difference.

          You can run tox -e lint and commit the results. See https://safir.lsst.io/set-up-from-template.html#format-code-with-black

          Show
          jsick Jonathan Sick added a comment - Finally: the newly created files could not be committed because isort said some imports were out of order. I used the package name "owl" in case that makes a difference. You can run tox -e lint and commit the results. See https://safir.lsst.io/set-up-from-template.html#format-code-with-black
          Hide
          rowen Russell Owen added a comment -

          The __init__.py files should not be needed with Python 3 in order to run code that is in the tests directory. Such files were needed in Python 2 but not in Python 3 in order to make a directory into a package. Russ Allbery: I understand you requested these files? If so, have you tried running the tests without them?

          Show
          rowen Russell Owen added a comment - The __init__.py files should not be needed with Python 3 in order to run code that is in the tests directory. Such files were needed in Python 2 but not in Python 3 in order to make a directory into a package. Russ Allbery : I understand you requested these files? If so, have you tried running the tests without them?
          Hide
          jsick Jonathan Sick added a comment -

          I've fixed the extra _all_ statements and the import order issue.

          Show
          jsick Jonathan Sick added a comment - I've fixed the extra _ all _ statements and the import order issue.
          Hide
          rra Russ Allbery added a comment - - edited

          The __init__.py files in the tests directory are required with Python 3. If they are not present, you cannot use the same test name in multiple directories. The errors that happen in this case are quite obscure and can be difficult to debug, so ensuring that __init__.py files always exist saves a lot of hassle. (FWIW, this was also Guido's recommendation internally at Dropbox when he was providing guidance on our Python layout: always put a __init__.py file in any directory containing Python source to avoid a lot of weird edge cases.)

          Show
          rra Russ Allbery added a comment - - edited The __init__.py files in the tests directory are required with Python 3. If they are not present, you cannot use the same test name in multiple directories. The errors that happen in this case are quite obscure and can be difficult to debug, so ensuring that __init__.py files always exist saves a lot of hassle. (FWIW, this was also Guido's recommendation internally at Dropbox when he was providing guidance on our Python layout: always put a __init__.py file in any directory containing Python source to avoid a lot of weird edge cases.)

            People

            Assignee:
            jsick Jonathan Sick
            Reporter:
            rowen Russell Owen
            Watchers:
            Jonathan Sick, Russ Allbery, Russell Owen
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                CI Builds

                No builds found.