Skip to content

Rename types inside MLContext as Catalogs #1796

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

Closed
TomFinley opened this issue Nov 30, 2018 · 2 comments
Closed

Rename types inside MLContext as Catalogs #1796

TomFinley opened this issue Nov 30, 2018 · 2 comments
Assignees
Labels
API Issues pertaining the friendly API good first issue Good for newcomers up-for-grabs A good issue to fix if you are trying to contribute to the project

Comments

@TomFinley
Copy link
Contributor

We had an idea in #949 to have "context" objects that enabled the easy and convenient creation of training algorithms. We eventually came to like this idea so much that it expanded considerably, until we came to think it actually a good idea that most components (whether trainers or not) could work through this object, to the point where eventually in in #1098, there was a "master" context, of which the formerly independent contexts became properties of that master context.

The newer things that were added were then called "catalogs," for example:

public TransformsCatalog Transforms { get; }

public ModelOperationsCatalog Model { get; }

However the old objects retained the type names they had back when they were independent.

public BinaryClassificationContext BinaryClassification { get; }

We ought to probably standardize the name of these things as being "catalogs." E.g., things in here with the type name suffix Context should change that to Catalog, things that are catalogs but aren't named that yet (DataOperations) should be standardized as well.

@TomFinley TomFinley added good first issue Good for newcomers API Issues pertaining the friendly API up-for-grabs A good issue to fix if you are trying to contribute to the project labels Nov 30, 2018
@mareklinka
Copy link
Contributor

Would the rename also apply to base classes? E.g. TrainContextBase in public sealed class BinaryClassificationCatalog : TrainContextBase? I would think that yes, those should be made consistent as well, just want to make sure.

The same thing then of course applies to method parameters (ctx -> catalog) and any mention in the documentation comments.

@TomFinley
Copy link
Contributor Author

Hi @mareklinka , good question, what you say makes sense to me, yes. I'd suppose most objects of this sort whether the abstract classes or the leaf classes would be named catalog instead of context.

@najeeb-kazmi najeeb-kazmi self-assigned this Jan 11, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Mar 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API Issues pertaining the friendly API good first issue Good for newcomers up-for-grabs A good issue to fix if you are trying to contribute to the project
Projects
None yet
Development

No branches or pull requests

3 participants