Update handling configuration items in template

[eerovaher]

Three updates have been made to the subpackage template.

1. It now recommends implementing private properties so that it would be possible to use the astropy configuration system at runtime, or to overwrite it entirely using class or instance attributes. See Fix changing simbad config items at runtime #2292 for an example of what this means in practice.
2. The template attributes that would overwrite the configuration items are no longer named as if they are constants, and the configuration item names match the attribute names.
3. It is now recommended to implement an __init__() method that would allow the user to conveniently create instances with non-default configuration attributes.

These updates, if merged, would specify the recommended API for any new subpackages, but would also serve as a guideline for updating the existing subpackages to converge on a more uniform API.

Motivation

The astropy configuration system provides many useful features, for example:

• users can make permanent changes through editing the configuration file,
• the set_temp() context manager makes it convenient to test different configuration item values,
• configuration item values and types are verified, so attempting to set an invalid value fails immediately.

astroquery should therefore use the astropy configuration system as much as possible, but users not familiar with it would benefit if an alternative way of changing configurable values would also be present.

The current template recommends creating class attributes which take their values from the configuration system, but the class attributes are created when the class is created, which happens when the module containing the class is executed for being imported. This means that changing the configuration items at runtime has no effect on the class attributes, which is the cause of many bugs in astroquery (see #544 and #2291, or for specific examples see e.g. #493 and #2099).

The new template proposed in this pull request would recommend the query classes to implement for every configuration item a corresponding instance attribute and private property. The private property would return the value of the instance attribute if a non-default value has been specified through an optional keyword argument to the __init__() method at instance creation or directly at a later time, otherwise it would return the value from the configuration system. This has the following benefits:

• users familiar with the astropy configuration system can use all of its features, including changing values at runtime,
• users not familiar with the configuration system can still make changes in a simple way,
• developers writing code that needs to access configurable values can always do so in a uniform way through the private property, which will take care of selecting either the instance attribute or the configuration item without needless code duplication or harming code readability.

[codecov]

Codecov Report

Merging #2341 (6a4027a) into main (c0700b8) will increase coverage by 0.01%.
The diff coverage is 83.33%.

❗ Current head 6a4027a differs from pull request most recent head 1ffb0d9. Consider uploading reports for the commit 1ffb0d9 to get more accurate results

@@            Coverage Diff             @@
##             main    #2341      +/-   ##
==========================================
+ Coverage   63.41%   63.43%   +0.01%     
==========================================
  Files         131      131              
  Lines       17043    17051       +8     
==========================================
+ Hits        10808    10816       +8     
  Misses       6235     6235              

Impacted Files	Coverage Δ
astroquery/template_module/core.py	65.21% <83.33%> (+4.56%)	⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[keflavich]

I'm not sure our users will all be familiar with the term "falsy" - maybe clarify, e.g., ("falsy" means in this case bool(url) is False)

[eerovaher]

The target audience for these comments are developers, but I added an explanation nonetheless.

[keflavich]

our developers are not necessarily experienced, and in particular, not necessarily experienced in python (we have many people coming from Java backgrounds, for example)

[bsipocz]

But then "falsy" is not a python specific term :) Anyway, I don't think it matters either way. Also, in my experience, many new modules are taking another similar module as a template rather than the "template" one, so we may even want to rethink the purpose of the template.

[eerovaher]

Does this pull request need a change log entry or has the no-changelog-entry-needed label been forgotten?

[bsipocz]

I don't think it needs one, but I leave the label off as a reminder for release time to revisit, if this next release indeed accretes multiple of the smaller infra refracture we may want to add one entry to cover those changes.

[eerovaher]

I have added a section to the opening post that explains the motivation for the changes this pull request proposes.