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

Global and User config, CLI config #72

Merged
merged 9 commits into from
Dec 9, 2024
Merged

Global and User config, CLI config #72

merged 9 commits into from
Dec 9, 2024

Conversation

sneakers-the-rat
Copy link
Collaborator

@sneakers-the-rat sneakers-the-rat commented Nov 12, 2024

This PR ports some stuff i have worked on in linkml over to miniscope-io.

Specifically we

  • Adds a configurable user_dir source of config - currently there is a place to set base_dir but no way to get config from that directory, but now one can define global config there too.
  • Adds other sources of config because why the hell not - currently one can use .env file, but this adds the ability to use (in order of priority, high to low)...
    • mio_config.yaml in current directory
    • pyproject.toml - the [tool.miniscope_io.config] table
    • mio_config.yaml in a custom user directory
    • mio_config.yaml in a fixed global directory
  • Adds cli commands to
    • create/set a user directory
    • show user/global configs
    • show locations of user and global config files

This also improves testing for configs, making fixtures to set each of the types of settings sources.

I know i have been like maniacal about config on this project, but i swear it's not just a pointless yak shave: as i said before, configuration is going to be the main point of interaction that people have with miniscope-io since it's an SDK not a full-blown GUI app, it might get bundled in with other programs, and it's the way that we're exposing as how to control the operation of the program (somewhat tautologically...). So it needs to be tight and evolvable.

This sets us up to make the configuration of devices more straightforward, where we might have something like mio device {device_type} create to create a default configuration for that device into {user_dir}/devices so that they can then customize it. being able to have flexible configuration from the system level down to individual projects (i.e. global config vs. the directory-specific configs) then we make it possible to bundle device configs along with experimental data, so one could just cd to a different directory and automatically use the configs for the project in that directory.


📚 Documentation preview 📚: https://miniscope-io--72.org.readthedocs.build/en/72/

@coveralls
Copy link
Collaborator

coveralls commented Nov 12, 2024

Pull Request Test Coverage Report for Build 12246079711

Details

  • 132 of 144 (91.67%) changed or added relevant lines in 5 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+1.0%) to 79.112%

Changes Missing Coverage Covered Lines Changed/Added Lines %
miniscope_io/cli/main.py 0 2 0.0%
miniscope_io/models/mixins.py 5 7 71.43%
miniscope_io/cli/config.py 61 69 88.41%
Files with Coverage Reduction New Missed Lines %
miniscope_io/models/mixins.py 1 89.17%
Totals Coverage Status
Change from base Build 12243905522: 1.0%
Covered Lines: 1337
Relevant Lines: 1690

💛 - Coveralls

@sneakers-the-rat
Copy link
Collaborator Author

thoughts on this? i am finding myself wanting to use some of these fixtures in #76

Copy link
Collaborator

@t-sasatani t-sasatani left a comment

Choose a reason for hiding this comment

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

Looks very helpful! I was always confused around this.

Comment on lines +26 to +34
* in the arguments passed to the class constructor (not user configurable)
* in environment variables like `export MINISCOPE_IO_LOG_DIR=~/`
* in a `.env` file in the working directory
* in a `mio_config.yaml` file in the working directory
* in the `tool.miniscope_io.config` table in a `pyproject.toml` file in the working directory
* in a user `mio_config.yaml` file in the user directory (see [below](user-directory))
* in the global `mio_config.yaml` file in the platform-specific data directory
(use `mio config global path` to find its location)
* the default values in the {class}`~miniscope_io.models.config.Config` model
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, making this explicit helps a lot.

Comment on lines +75 to +77
Implement setting values from CLI.

For now, please edit the configuration files directly.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is also related to my comments in #76, but I'm not sure which is easier between these two (though it wouldn't hurt to have these two work nicely). I feel that using text editors could be easier than using CLI for our target users (~undefined), but I am unsure.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

totally ya. after seeing you working with the cli setting config values i think that would be really nice to have, and that should also edit the relevant files, so they should be made to be equivalent imo if we do set config values from the cli - i.e. editing from cli should change the value in the config files as if one were to edit it with a text editor

Comment on lines +1 to +3
"""
CLI commands for configuration
"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks very helpful.

@sneakers-the-rat
Copy link
Collaborator Author

well i sorta merged these backwards and made this much harder for myself.... my bad

@sneakers-the-rat
Copy link
Collaborator Author

I think that having a separate devices configs directory is probably more complex than we want. having a single flat id-based system where the user can structure it into whatever they want within configs is probably easier than having to keep track of where files are stored. gonna remove that part (it's not used anyway in this PR)

…el, since that means that when merging configs from multiple sources we end up taking default values from higher priority sources when unset
@sneakers-the-rat
Copy link
Collaborator Author

sneakers-the-rat commented Dec 9, 2024

Had to change some minor behavior here - now we no longer explicitly propagate log levels down from the base level value to level_file and level_stdout - the init_logger value handles that. This is because when multiple config sources are read, then the model instance from the highest priority source will fill those values in from the defaults, and then they override explicitly set values in lower priority sources. This way makes the config model a bit more explicit representation of the configs with less magic, which is always good. i checked that the only place those values were read was within the init_logger anyway, so should be function-neutral change.

also updated the test for setting log levels from being just via .env to all modes of config, since we have those fixtures now.

@sneakers-the-rat
Copy link
Collaborator Author

added some tests and we should be good to go

@sneakers-the-rat sneakers-the-rat merged commit fd3fee9 into main Dec 9, 2024
16 checks passed
@sneakers-the-rat sneakers-the-rat deleted the config-dir branch December 13, 2024 03:28
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.

3 participants