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

delete config #408

Merged
merged 3 commits into from
Apr 4, 2024
Merged

delete config #408

merged 3 commits into from
Apr 4, 2024

Conversation

stan-dot
Copy link
Contributor

No description provided.

@stan-dot stan-dot self-assigned this Mar 28, 2024
@stan-dot stan-dot linked an issue Mar 28, 2024 that may be closed by this pull request
@callumforrester
Copy link
Contributor

Tests that check the config is valid also need to be removed

Copy link

codecov bot commented Apr 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.41%. Comparing base (fb6fd0c) to head (4b480c9).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #408      +/-   ##
==========================================
- Coverage   89.52%   89.41%   -0.12%     
==========================================
  Files          43       43              
  Lines        1805     1805              
==========================================
- Hits         1616     1614       -2     
- Misses        189      191       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@callumforrester callumforrester left a comment

Choose a reason for hiding this comment

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

LGTM

@stan-dot stan-dot merged commit dba0f25 into main Apr 4, 2024
25 checks passed
@stan-dot stan-dot deleted the 357-remove-example-configs-from-repo-root branch April 4, 2024 07:36
ZohebShaikh pushed a commit that referenced this pull request May 7, 2024
@tpoliaw
Copy link
Contributor

tpoliaw commented Jun 27, 2024

There are still references to this file in docs and a(n unused) test fixture that relies on it. It would be good if the docs could be updated to either include some example config or link to somewhere some examples can be found.

@stan-dot
Copy link
Contributor Author

hmmm, please say which fille

@tpoliaw
Copy link
Contributor

tpoliaw commented Jun 27, 2024

Sorry, the defaults.yaml file

(⇅ 3) v3.10.13 (blueapi) /scratch/athena/blueapi (main)[*?]
❯ rg -C2 'defaults.yaml'
tests/test_config.py-53-@pytest.fixture
tests/test_config.py-54-def default_yaml(package_root: Path) -> Path:
tests/test_config.py:55:    return package_root.parent.parent / "config" / "defaults.yaml"
tests/test_config.py-56-
tests/test_config.py-57-
--
docs/how-to/configure-app.md-5-can be passed to the `blueapi` command.
docs/how-to/configure-app.md-6-
docs/how-to/configure-app.md:7:An example of this yaml file is found in `config/defaults.yaml`, which follows
docs/how-to/configure-app.md-8-the schema defined in `src/blueapi/config.py` in the `ApplicationConfig`
docs/how-to/configure-app.md-9-object.

@stan-dot
Copy link
Contributor Author

would putting it inside the ConfigLoader docstring be suitable?

C = TypeVar("C", bound=BaseModel)


class ConfigLoader(Generic[C]):
    """
    Small utility class for loading config from various sources.
    You must define a config schema as a dataclass (or series of
    nested dataclasses) that can then be loaded from some combination
    of default values, dictionaries, YAML/JSON files etc.
    """

    _schema: type[C]
    _values: dict[str, Any]

src/blueapi/config.py lines 115 and later?

@tpoliaw
Copy link
Contributor

tpoliaw commented Jul 1, 2024

That would be good as well, but I think having examples in the docs without having to read through the code would be helpful for setting up new environments.

@stan-dot stan-dot mentioned this pull request Jul 1, 2024
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.

Remove example configs from repo root
3 participants