Closed tjarkvandemerwe closed 1 month ago
Oke, ben weer wat verder en de manier waarop ik het nu heb gedaan vind ik nog niet heel mooi. Voordeel is al wel dat je niet meer variabelen hoeft over te tikken, maar ik denk dat het mooier is om het environment van de functie even op te slaan, mee te geven aan de prompt_wrap constructor, en daar dan per functie het environment mee te geven. Zodat je het niet bij het maken van iedere wrap zelf per functie hoeft te doen. Dit is nog een TODO.
Top, inject env is inderdaad wat cleaner dan
Zou denk ik wel weer de prompt wraps een list maken binnen het object, dat is m.i. wat overzichtelijker (en biedt ook ruimte om nog andere dingen aan het object te hangen). Functies aan het object hangen vind ik zelf ook wel mooi, dan kan je als gebruiker gemakkelijk via '$'-operator zien wat er allemaal mogelijk is met het object, zonder verdere documentatie te hoeven raadplegen. Nadeel is wel dat je er dan vgm niet gemakkelijk roxygen-documentatie voor kan aanbieden.
@tjarkvandemerwe Ik heb nu weer een wat stabielere versie op de main branch gezet, misschien binnenkort even paar dingen bespreken qua hoe we de architectuur willen.
Injecteren van variabelen bleek niet nodig omdat dat eigenlijk standaard al gebeurt; dus heb de functie nu weer weggehaald.
Qua generics twijfel ik nog een beetje, we kunnen overal een .default-methode voor maken die poogt er een prompt-object van te maken via de constructor; en dan een functie voor als wel een prompt object is meegegeven. Maar dat is wel een beetje omslachtig qua hoeveelheid regels code voor wat eigenlijk gewoon 1 call naar de constructor is (die ook dienst doet als een validator, dus wil je misschien sowieso callen ook als je wel het juiste type krijgt). Voordeel van generics is dan weer wel dat je per klasse een overzicht kan krijgen van alle beschikbare methoden.
Ik denk dat 'prompt' of 'tidyprompt' als objectnaam misschien mooier kan zijn dan 'prompt_list'.
Verder nog wat tests toegevoegd aan het package, met 'testthat', kan handig zijn.
Ik snap de voordelen van functies aan object hangen. Idd gemakkelijk om met $ even te kijken wat er kan.
Het heeft idd het nadeel van de documentatie. Maar het belangrijkste probleem vind idd daarbij dat het gedrag niet helemaal standaard is in R, dus als gebruiker moet je wennen tov functie(x) of x |> functie. Dit maakt het net iets moeilijker voor nieuwe gebruikers om zelf objecten te maken oid. En een kleiner punt is dat het object wel behoorlijk onoverzichtelijk wordt, als ik de str() of gewoon de print van de list bekijk staat er wel veel met ook alle functies erbij.
Top van het injecteren van variabelen! Dat had ik nog niet door dat het env zo mooi meekomt standaard.
Generics: eens, ik denk dat er twee stappen zijn waarbij we zoiets doen, bij toevoegen van een wrap aan een prompt. En mogelijk bij send prompt. Dat zijn de directe interfaces waar je verwacht te werken met een prompt object. En je miss een character en prompt variant van wilt maken. En de rest van de functies zijn variaties op deze interface en gebruiken altijd de wrap functie die ook deze functionaliteit heeft.
Eens met prompt en tidyprompt!
Tests zijn nice! Mogelijk voor testen nog even een dummy llm connectie object maken.
In plaats van kopietjes van variabelen meegeven als argumenten in de wrap creation geef ik nu het environment mee met de functie. Zodat in het voorbeeld van de answer_as_integer de min en max ed allemaal worden opgeslagen naast de functies die gecreëerd worden.
commit: 0c9bf09c015b1d9b62e6113fa85924adf48f8aee