Skip to content

Config pkg, take 2 - #73

Merged
eteq merged 65 commits into
astropy:masterfrom
eteq:config-pkg
Nov 26, 2011
Merged

eteq merged 65 commits into
astropy:masterfrom
eteq:config-pkg

Conversation

@eteq

@eteq eteq commented Nov 21, 2011

Copy link
Copy Markdown
Member

This is a re-work of the remote data and configuration package heavily modified based on feedback in pull request #35 . The important major changes:

  • Both the cache and config directories now use ~/.astropy/config and ~/.astropy/cache, and the XDG_CONFIG_HOME and XDG_CACHE_HOME environment variables can be used to override these.
  • The remote data-fetching scheme now works completely on hashes internally - no need to worry about local data names
  • Data provided in the source code tree can now be accessed using the get_data function as though it were in the local data directory.
  • The config system is much enhanced and is now built around a ConfigurationItem object - see the documentation section I wrote that summarizes the typical use case, or look at the top of data.py in this pull request.

Things that are missing but can be easily added later:

  • Code to auto-generate the configuration files when astropy is installed (although see the astropy.config.configs._generate_all_config_items function - that basically just has do be plugged into the setup)
  • A way to lookup environment variables in place of the configuration files (Support overriding config options via environment variables #48)
  • Better documentation for the data system

@eteq

eteq commented Nov 23, 2011

Copy link
Copy Markdown
Member Author

@mdboom - Thanks for the detailed feedback! Everything that's not commented on above should be fixed in the version I pushed up just now.

Regarding the auto-writing of the first file: I'm very much against the by-hand approach, because I'm certain there will be lots of problems with people forgetting to do so. I tried to write this system here so that the output config file is still pretty nice looking with the necessary comments and so on.

So yeah, I think the best approach is to do something like setup.py test does - my thought was that this could be a new setup.py command, allowing people to wipe their current config and startover if they provide the appropriate option, but otherwise it only creates a new one if it isn't there already. Then we could set it up so that the setup.py install and setup.py develop commands run that command (in the non-overwriting form) when they finish everything else.

@eteq

eteq commented Nov 23, 2011

Copy link
Copy Markdown
Member Author

@astrofrog,@iguananaut - you two both had a variety of comments on the last pull request, so I'll wait for you two to either give feedback or say it looks good before merging anything.

@hamogu, @phn - you may also be interested, as you both had a few comments in the old pull request.

@eteq

eteq commented Nov 23, 2011

Copy link
Copy Markdown
Member Author

This are notes mostly for myself, as I will make issues for each of the not-yet-completed items after this pull request is done:

  • Provide some sort of progress feedback when downloading remote data
  • implement data.astropy.org server
  • ?move the function that locates it's caller's module to utils?
  • ?unified file-locking scheme?

Comment thread astropy/config/configs.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo (confoibj)

@astrofrog

Copy link
Copy Markdown
Member

I haven't read everything in detail, but it seems fine to me. In any case, I'm sure we will need to tweak things once we start actually making use of the config utilities.

@mdboom

mdboom commented Nov 23, 2011

Copy link
Copy Markdown
Contributor

@eteq: As for writing the defaults -- I think I wasn't clear. setup.py can not be responsible for writing the file to the user's home directory. The person running setup.py may be a sysadmin installing on a shared server, which is then run by users in multiple locations. These users probably don't have access to setup.py or at best won't know where to look for it.

What I meant to say is that setup.py build (why make it an additional step?) would create the template file, and install that along with astropy. Some mechanism on importing astropy once installed would check the user's home directory, and if finding no config file, copy the template there. We can also provide an easy-to-find function to reset all configuration files, too.

A simpler solution would be to not generate and install a template file, but simply generate it when needed. This has the downside that all of astropy needs to be imported upon first usage -- maybe not a big deal at present, though.

@mdboom

mdboom commented Nov 23, 2011

Copy link
Copy Markdown
Contributor

Looks good to me. Once this is merged, there are some places I'd like to start using it :)

@eteq

eteq commented Nov 23, 2011

Copy link
Copy Markdown
Member Author

@mdboom - ah, I get what you mean now. Given the range of options there, I'll make sure to put it up as an issue and there can be a later pull request to deal with how to actually organize the generation.

@embray

embray commented Nov 23, 2011

Copy link
Copy Markdown
Member

The only problem I see with this is that autogenerating a config file means every module has to be imported once to make sure all config options are found. I don't think there's a way around that though. Honestly, I'm not against including a template config file (or files) in the source code--if a new option is added it would just have to be added to the template. There could even be code to generate it.

On the other hand, as long as the generation only has to happen once that's fine too. There should be a function (if there isn't already) to update a user's config file with new options while preserving existing settings.

@eteq

eteq commented Nov 26, 2011

Copy link
Copy Markdown
Member Author

All the remaining changes seem to be targets for later pull requests, so I'll merge this now. Thanks for all the helpful feedback!

eteq added a commit that referenced this pull request Nov 26, 2011
Implementation of config package - includes both configuration file and data-access functionality.
@eteq
eteq merged commit e6c4a34 into astropy:master Nov 26, 2011
keflavich pushed a commit to keflavich/astropy that referenced this pull request Oct 9, 2013
Implementation of config package - includes both configuration file and data-access functionality.
@eteq
eteq deleted the config-pkg branch May 3, 2015 17:16
astrofrog referenced this pull request in astrofrog/astropy Nov 23, 2016
Fix issues when spacing is not a multiple of base spacing
astrofrog pushed a commit to astropy/sphinx-astropy that referenced this pull request Jan 29, 2018
Implementation of config package - includes both configuration file and data-access functionality.
@pllim pllim mentioned this pull request Oct 24, 2022
10 tasks
jeffjennings pushed a commit to jeffjennings/astropy that referenced this pull request Jul 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants