PEP 440 documents Python versioning guidelines in detail. In particular this is a good place to read the details about 1.2.3a1, 2.3.1b2, 1.3.4.dev45.

Note that it recommends .dev, not dev.

It may be time to update the style guide and documentation for this package. Some things that would be worth mentioning:

• bump2version instead of bumpversion.
• Implementation of #18.
• How to best use pip for development (-e option, PEP-517/518).
• Expand the GitHub workflows section.
• Virtualenvs
• Stronger language about deprecating Python 2?
• ZenHub/Agile best practices
• Squash commits

The coding standards associated with the template make a specific recommendation for file headers, however, it is not clear where these recommendations come from, or how these values would be read or set in practice.

Also, the specific line

# encoding: utf-8

is more commonly

# -*- coding: utf-8 -*-

Finally, by recommending

#!/usr/bin/env python

you're basically saying that every Python module file will be executable. I don't think this is recommended practice. The #! line only belongs in truly executable scripts that will be installed in a bin/ directory.

Python and pip are slowly rolling out the implementation for PEP 517 and 518. Here's an article describing it.

In particular this allows using tools such as poetry that provide a really nice workflow and remove many of the inconveniences of using requirements.txt, setup.py, etc.

The disadvantages are that the system is still being deployed (although pip already supports it) and that removes old-style installation from source with setup.py. The new system seems to be take-or-leave it and it's probably not worth it to try to implement it while keeping compatibility with the previous build system.

.flake8, tox.ini, .bumpversion.cfg, and .coveragerc can be all merged into a single setup.cfg. PyLint will also provide support for .pylintrc in setup.cfg soon. If we do #17 we should move as much as possible to pyproject.toml.

The coding standards recommend

from __future__ import division
from __future__ import print_function
from __future__ import absolute_import

but oddly, not

from __future__ import unicode_literals

That one is actually the most important for catching 2 to 3 transition problems.

The logger will not import in python2

The module file has this line:

# conflict $product

When not commented out, this line prevents two versions of the same product from being loaded simultaneously. This is a good thing.

I’ve encountered the following error a few times in the past. Its reocurrence makes me think there might be a bug in python_template. It happens when I use logger.warning with a completely different logger than the logger utils/logger.py included with the template, yet it throws an error in utils/logger.py. If I change logger.warning to logger.info it works as expected with the correct logger.
The offending line is
self.logger.warning('Encountered unsupported install urls: {}. Skipping...'.format(diff))

sdss_install.utils.logger - ERROR - logger.py - line 172 - Uncaught exception
Traceback (most recent call last):
  File "/Users/ben/Compute/Python/sdss_install/idlutils/bin/sdss_install", line 50, in <module>
    install.install_external_dependencies()
  File "/Users/ben/Compute/Python/sdss_install/idlutils/python/sdss_install/install/Install.py", line 303, in install_external_dependencies
    if install_urls: self.install_external_products(install_urls=install_urls)
  File "/Users/ben/Compute/Python/sdss_install/idlutils/python/sdss_install/install/Install.py", line 321, in install_external_products
    'Skipping these.')
  File "/Users/ben/Compute/Python/sdss_install/idlutils/python/sdss_install/utils/logger.py", line 194, in warning
    if category in self.warning_registry:
AttributeError: 'MyLogger' object has no attribute 'warning_registry'
Traceback (most recent call last):
  File "/Users/ben/Compute/Python/sdss_install/idlutils/bin/sdss_install", line 50, in <module>
    install.install_external_dependencies()
  File "/Users/ben/Compute/Python/sdss_install/idlutils/python/sdss_install/install/Install.py", line 303, in install_external_dependencies
    if install_urls: self.install_external_products(install_urls=install_urls)
  File "/Users/ben/Compute/Python/sdss_install/idlutils/python/sdss_install/install/Install.py", line 321, in install_external_products
    'Skipping these.')
  File "/Users/ben/Compute/Python/sdss_install/idlutils/python/sdss_install/utils/logger.py", line 194, in warning
    if category in self.warning_registry:
AttributeError: 'MyLogger' object has no attribute 'warning_registry'
[ben@cypher:sdss]$

When setting up the template, get the default full_name and email from the git configuration.

Hi @albireox,

I haven't use the python_template in a very long while, so here's my attempt and the error received, if you can have a look and let me know if I did something incorrect, or if I should try it in an older miniconda than 3.11?

$ cookiecutter https://github.com/sdss/python_template.git
  [1/14] full_name (John Doe): Joel Brownstein
  [2/14] email ([email protected]): [email protected]
  [3/14] package_name (myproject): allspec
  [4/14] repo_name (allspec):
  [5/14] pip_name (sdss-allspec):
  [6/14] version (0.1.0-alpha.0):
  [7/14] year (2024):
  [8/14] short_description (Simple Template package for creating SDSS Python projects): Simple package to generate the allspec file which would extract sdss_ids from the pipeline's database including boss and apogee spectra, or all spectra with an sdss_id
  [9/14] Select packaging_system
    1 - setuptools
    2 - poetry
    Choose from [1/2] (1):
  [10/14] Select sphinx_template
    1 - sphinx-bootstrap
    2 - alabaster
    Choose from [1/2] (1):
  [11/14] Select use_releases
    1 - no
    2 - yes
    Choose from [1/2] (1):
  [12/14] Select create_git_repo
    1 - yes
    2 - no
    Choose from [1/2] (1):
  [13/14] Select exists_on_github
    1 - no
    2 - yes
    Choose from [1/2] (1):
  [14/14] github_organisation (sdss):
Traceback (most recent call last):
  File "/var/folders/dz/0sxcvbqd48700v2k7mnc5b8w0000gn/T/tmp46a2p62_.py", line 53, in <module>
    @invoke.task
     ^^^^^^^^^^^
  File "/software/pkg/miniconda/3.11/lib/python3.11/site-packages/invoke/tasks.py", line 331, in task
    return klass(args[0], **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/software/pkg/miniconda/3.11/lib/python3.11/site-packages/invoke/tasks.py", line 76, in __init__
    self.positional = self.fill_implicit_positionals(positional)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/software/pkg/miniconda/3.11/lib/python3.11/site-packages/invoke/tasks.py", line 167, in fill_implicit_positionals
    args, spec_dict = self.argspec(self.body)
                      ^^^^^^^^^^^^^^^^^^^^^^^
  File "/software/pkg/miniconda/3.11/lib/python3.11/site-packages/invoke/tasks.py", line 153, in argspec
    spec = inspect.getargspec(func)
           ^^^^^^^^^^^^^^^^^^
AttributeError: module 'inspect' has no attribute 'getargspec'. Did you mean: 'getargs'?
ERROR: Stopping generation because post_gen_project hook script didn't exit successfully
Hook script failed (exit status: 1)

RTD has changed its configuration file definition. See https://docs.readthedocs.io/en/stable/config-file/v2.html. The file now needs to be .readthedocs.yml

• Remove travis?
• Update RTD configuration file.
• Always use pyproject.toml but contents will depend on whether the user selects poetry or setuptools.
• Keep poetry as an option but if the user selects setuptools add a post-process step to create a lockfile using pip-tools. Document how to maintain the lockfile.
• Update test GitHub workflow. Split test into linting and unittesting.
• Add release GitHub workflow. Build wheels and source and push to PyPI. Document trusted publishers.
• Add (optional?) workflow to build RTD.
• Make default linter ruff. Add black to dev dependencies (maybe also darker, choose your own grayscale?)
• Change Sphinx to furo. Remove other options.
• Add Sphinx extensions: copy button, inline tabs, sphinx-autodoc-typehints, myst-parser.
• Add CLI hook. If so, use click if so. Add documentation hook with sphinx-click.
• Remove releases. Use markdown changelog.
• (Maybe) create bash script to automate release, bump version, etc.
• Add auto-refresh to sdsstools docs.build.
• In sdsstools improve config merging using deepmerge
• Check that the config file can be yml or yaml.
• Add documentation to use rich handler in the logger.
• Remove .pylintrc.
• Add from __future__ import annotations.
• Use type hinting in all files.
• Make sure that docs API works with type hints.
• Remove class example in tests.
• Examples for mocking the SAS/download files in GitHub actions. Maybe a separate package?
• Examples/fixtures for unittest mocking (mocking API calls, monkeypatching vs mocking, use of sdss_access in tests).
• Remove top-level etc/, cextern, bin/ directory.
• (Maybe) Rename python/ to src/.
• Update coding standards to reflect new tools and procedures in the template.
• Add section to coding standards about recommended editor (especially VSCode) settings, extensions (ruff, remote SSH, etc.)
• Think about what SDSS dependencies (tree, access, datamodel) we want to include by default.
• Talk about SDSS products (tree, access, datamodel) in the coding standards?
• Update Sphinx index page.

The logger has some code that overrides warnings.show_warning to our own show_warning which uses the logging system to show the warning with some coloured formatting.

Recently I have started to see the problems with this approach. The main issue is that when you have more than one library that use the same system, warnings.show_warning is effectively redirected to the logger of the last library imported (and the last log activated). The warning will probably still show up, since all loggers will somehow display the warning, but if the warning is being logged to a file it will end up in the log file of a logger that is not its own.

Some possible solutions are:

1. Disable the warning redirection. The warning will show up unformatted and won't be logged.
2. Disable the warning redirection and use log.warning instead, which can be formatted and logged.
3. Move the custom show_warning to a super package (something like sdss_helpers) that keeps a registry of all the loggers and is able to redirect each warning to the correct one (for instance, all warnings that inherit from TreeWarning will be redirected to the tree logger).

2. Is a possible solution. It means that users won't be able to filter warnings using warnings.filterwarning or similar. I don't like 3) because it would mean introducing a dependency for all SDSS packages.

For now I think we should go with 1) and disable the warnings redirection. Repo owners can decide whether they want to use log.warning instead.

In the file python_template/{{cookiecutter.repo_name}}/etc/{{cookiecutter.package_name}}.module
shouldn’t the line
module-whatis {{"Sets up $product/$version in your environment."}}
instead be
module-whatis "{{Sets up $product/$version in your environment.}}"
so the quotation marks don’t get replaced in the sdss_install substitution?

This issue is similar to the one discussed here
pypa/setuptools#1136
and here
pypa/pip#5732
It seems to only effect Python 2.7. Commenting out the line - '2.7' in .travis.yml fixes the problem. However, some repos still use Python 2.7, such as sdss_install through sdss4install, so this is not an ideal workaround.

The change log file is at the top-level of the output directory. However, if it were moved inside the docs/sphinx directory, it would be visible in the Sphinx/ReadTheDocs documentation, which is often useful.

Right now invoke deploy deploys whatever current version is checked out. This means that unless one is careful it's easy to deploy the master branch by mistake instead of the tagged version. Moreover, it should be disallowed to deploy a version that is not properly tagged.

invoke deploy should be rewritten to require a tag argument. The code should then checkout that tag, deploy it, and revert to HEAD.

Should bumpversion be installed at the start of the documentation at http://sdss-python-template.readthedocs.io/en/latest/ in the same way that invoke is installed? Later in the documentation bumpversion is referred to (in a similar way that invoke is), but bumpversion may not be installed.

The documentation advises that:

Available levels are .debug, .info, .error, and .critical. For warnings, use warnings module.

But in fact logging an .error level does nothing, logging a .critical raises an exception (trace below), and there are leftovers from when "warnings" were available.

In [3]: log.critical("what")
---------------------------------------------------------------------------
UnboundLocalError                         Traceback (most recent call last)
<ipython-input-3-53d8881f152d> in <module>()
----> 1 log.critical("what")

~/anaconda2/envs/py36/lib/python3.6/logging/__init__.py in critical(self, msg, *args, **kwargs)
   1351         """
   1352         if self.isEnabledFor(CRITICAL):
-> 1353             self._log(CRITICAL, msg, args, **kwargs)
   1354 
   1355     fatal = critical

~/anaconda2/envs/py36/lib/python3.6/logging/__init__.py in _log(self, level, msg, args, exc_info, extra, stack_info)
   1440         record = self.makeRecord(self.name, level, fn, lno, msg, args,
   1441                                  exc_info, func, extra, sinfo)
-> 1442         self.handle(record)
   1443 
   1444     def handle(self, record):

~/anaconda2/envs/py36/lib/python3.6/logging/__init__.py in handle(self, record)
   1450         """
   1451         if (not self.disabled) and self.filter(record):
-> 1452             self.callHandlers(record)
   1453 
   1454     def addHandler(self, hdlr):

~/anaconda2/envs/py36/lib/python3.6/logging/__init__.py in callHandlers(self, record)
   1512                 found = found + 1
   1513                 if record.levelno >= hdlr.level:
-> 1514                     hdlr.handle(record)
   1515             if not c.propagate:
   1516                 c = None    #break out

~/anaconda2/envs/py36/lib/python3.6/logging/__init__.py in handle(self, record)
    861             self.acquire()
    862             try:
--> 863                 self.emit(record)
    864             finally:
    865                 self.release()

~/research/projects/astra/python/astra/utils/logger.py in colored_formatter(record)
     95             message = '{} {}'.format(color_text(warning_text, ''), warning_category_colour)
     96 
---> 97     sys.__stdout__.write('{}{}\n'.format(header, message))
     98     sys.__stdout__.flush()
     99 

UnboundLocalError: local variable 'header' referenced before assignment