Skip to content

provide public accessors for ModelConfigurations from registries#219

Merged
davidkoski merged 1 commit intoml-explore:mainfrom
toffaletti:expose-registered-models
Mar 4, 2025
Merged

provide public accessors for ModelConfigurations from registries#219
davidkoski merged 1 commit intoml-explore:mainfrom
toffaletti:expose-registered-models

Conversation

@toffaletti
Copy link
Contributor

Added public accessors for getting a list of registered ModelConfigurations. Would be nice to standardize these eventually.

@toffaletti toffaletti force-pushed the expose-registered-models branch from c115570 to fd3fa89 Compare March 1, 2025 01:42
}

@MainActor
public static var models: Dictionary<String, ModelConfiguration>.Values {
Copy link
Collaborator

@davidkoski davidkoski Mar 1, 2025

Choose a reason for hiding this comment

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

I don't think we should expose a type like this -- it leaks the implementation.

I wonder if this can be something like some RandomAccessCollection<ModelConfiguration>. If not, it should probably just be [ModelConfiguration] and we rewrap the return values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to make it Sendable too

@toffaletti toffaletti force-pushed the expose-registered-models branch from fd3fa89 to 5e3fdd7 Compare March 3, 2025 19:52
@toffaletti toffaletti force-pushed the expose-registered-models branch from 5e3fdd7 to 13c3e64 Compare March 3, 2025 19:57
@davidkoski davidkoski merged commit 06c825a into ml-explore:main Mar 4, 2025
3 checks passed
@toffaletti toffaletti deleted the expose-registered-models branch March 4, 2025 20:27
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.

2 participants