Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use traitlets for our configuration. #16476

Open
nstarman opened this issue May 20, 2024 · 5 comments
Open

Use traitlets for our configuration. #16476

nstarman opened this issue May 20, 2024 · 5 comments
Labels
config external PRs and issues related to external packages vendored with Astropy (astropy.extern) Feature Request Refactoring

Comments

@nstarman
Copy link
Member

nstarman commented May 20, 2024

What is the problem this feature will solve?

We currently bundle configobj with Astropy and use it to do our configuration. It's old, not well supported / actively developed, and doesn't do nice things like load configurations from JSON files.

Describe the desired outcome

We replace configobj with traitlets, which is the IPython and Jupyter configuration system (but that depends on neither). It's stable, well-supported, actively developed, supports a lot of modern conveniences, and has 0 dependencies.

Additional context

See the Design Requirements!
I wish our config system supported this.

Here are the main requirements we wanted our configuration system to have:

  • Support for hierarchical configuration information.

  • Full integration with command line option parsers. Often, you want to read
    a configuration file, but then override some of the values with command line
    options. Our configuration system automates this process and allows each
    command line option to be linked to a particular attribute in the
    configuration hierarchy that it will override.

  • Configuration files that are themselves valid Python code. This accomplishes
    many things. First, it becomes possible to put logic in your configuration
    files that sets attributes based on your operating system, network setup,
    Python version, etc. Second, Python has a super simple syntax for accessing
    hierarchical data structures, namely regular attribute access
    (Foo.Bar.Bam.name). Third, using Python makes it easy for users to
    import configuration attributes from one configuration file to another.
    Fourth, even though Python is dynamically typed, it does have types that can
    be checked at runtime. Thus, a 1 in a config file is the integer '1',
    while a '1' is a string.

  • A fully automated method for getting the configuration information to the
    classes that need it at runtime. Writing code that walks a configuration
    hierarchy to extract a particular attribute is painful. When you have
    complex configuration information with hundreds of attributes, this makes
    you want to cry.

  • Type checking and validation that doesn't require the entire configuration
    hierarchy to be specified statically before runtime. Python is a very
    dynamic language and you don't always know everything that needs to be
    configured when a program starts.

@neutrinoceros
Copy link
Contributor

Vendoring something that isn't actively developed indeed sounds worse than adding a dependency on a package that doesn't have any.
Are you proposing to vendor traitlets or to add it as a dependency ? If we vendor it, could it create incompatibilities with IPython/Jupyter at runtime (I assume not) ? If we depend on it, how likely is it that future versions might break astropy in a significant way ?

@nstarman
Copy link
Member Author

Are you proposing to vendor traitlets or to add it as a dependency ?

Dependency.

If we depend on it, how likely is it that future versions might break astropy in a significant way ?

Nothing can be promised, but it's been stable. And the config system in particular is stable because it's isolated: we would only use it for our config.

@eerovaher
Copy link
Member

Could switching to traitlets help with #9893?

@pllim pllim added Refactoring external PRs and issues related to external packages vendored with Astropy (astropy.extern) config labels May 21, 2024
@pllim
Copy link
Member

pllim commented May 21, 2024

I think you are talking about gutting out something fundamental in astropy.config. I see your point but the impact is very high, as the configuration system is also widely used downstream. I wonder if this warrants an APE.

@nstarman
Copy link
Member Author

nstarman commented May 21, 2024

@eerovaher.

Could switching to traitlets help with #9893?

Yes! Traitlets supports custom types. It's quite easy to do (https://traitlets.readthedocs.io/en/stable/defining_traits.html)

@pllim

I think you are talking about gutting out something fundamental in astropy.config.

Yes.

I see your point but the impact is very high.

🎉

as the configuration system is also widely used downstream.

There's two different uses:

  1. configuring Astropy itself. This is a breaking change in Astropy so downstream libraries would have to modify their code.

  2. Downstream libraries building their own config by inheriting from astropy.config. The current config in extern has seen very little change so it would be relatively easy to spin it out into a standalone legacy library that we basically don't touch except to add a deprecation warning. Then libraries that are building their own config on top of astropy.config would just have a trivial change to their import, e.g. legacy_astropy_config_system (or whatever we name it).

I wonder if this warrants an APE.

Sadly, probably.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config external PRs and issues related to external packages vendored with Astropy (astropy.extern) Feature Request Refactoring
Projects
None yet
Development

No branches or pull requests

4 participants