Tiktoken icon indicating copy to clipboard operation
Tiktoken copied to clipboard

Suggestion regarding the style of API

Open ddobric opened this issue 1 year ago • 3 comments

Hi guys,

I love your library—great job. However, I have some suggestions about the API you used in the last package.

Designing the API like this one is not really best way:

var encoder = ModelToEncoder.For("gpt-4o"); // I suggest using intuitive object names and method names.

The following is widely more acceptably standard in the software industry for many reasons:

`var encoder = TokenEncoder.Create("gpt-4o");

or

var encoder = new TokenEncoder.For("gpt-4o");

You can also use the Tiktoken or Tiiktoken encoder. ModelEncoder is not an intuitive name. We are not encoding Models. Tokenizer is not a concept of model by its original paper. We should stay consistent with the references which we use.

Another suggestion related to providing the "string" ("gpt-xy") is ok, but some kind of enum or similar round string would be helpful.

var encoder = new TokenEncoder.For(Models.Gpt4o);

Hope this helps.

ddobric avatar Aug 01 '24 12:08 ddobric

Hi. There is a small problem with Encoder.For/Encoder.Create - because this library is split into several packages, and the base package with abstractions for Encoder knows nothing about implementations for it, so I created a new class ModelToEncoder in the meta package, which already combines the main used Encoders. And therefore there is no way to somehow change the TokenEncoder class in it

Maybe I should rename it to Model2Encoder?

HavenDV avatar Aug 01 '24 16:08 HavenDV

@HavenDV how about Encoders.ForModel(...) / Encoders.TryForModel(...)?

mikethea1 avatar Aug 14 '24 16:08 mikethea1

There will be problems because it is part of the namespace.

HavenDV avatar Aug 14 '24 19:08 HavenDV