-
Notifications
You must be signed in to change notification settings - Fork 5
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
490 implement coexistence of created table with and without keys #494
Merged
marcboulle
merged 5 commits into
dev
from
490-implement-coexistence-of-created-table-with-and-without-keys
Jan 2, 2025
Merged
490 implement coexistence of created table with and without keys #494
marcboulle
merged 5 commits into
dev
from
490-implement-coexistence-of-created-table-with-and-without-keys
Jan 2, 2025
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
marcboulle
force-pushed
the
490-implement-coexistence-of-created-table-with-and-without-keys
branch
2 times, most recently
from
December 17, 2024 08:09
201cee1
to
71b1147
Compare
popescu-v
reviewed
Dec 20, 2024
popescu-v
reviewed
Dec 20, 2024
popescu-v
reviewed
Dec 20, 2024
popescu-v
reviewed
Dec 20, 2024
popescu-v
reviewed
Dec 20, 2024
popescu-v
reviewed
Dec 20, 2024
popescu-v
reviewed
Dec 20, 2024
popescu-v
reviewed
Dec 20, 2024
popescu-v
reviewed
Dec 20, 2024
popescu-v
reviewed
Dec 20, 2024
popescu-v
requested changes
Dec 20, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few typos and questions.
Contraintes sur les cle dans le cas de la creation de table - une table creee peut etre associee a un dictionnaire ayant ou non des champs cles - par exemple, si l'on construit une Table a partir d'un champs Json, on n'a pas besoin de cle - par contre, il est interdit de construire une table externe, geree par un dictionnaire Root - en effet, dans ce cas, Root est un indicateur de table externe explicite, ce qui implique un fichier de donnees dedie En definitive, un schema multi-table cree ne peut etre Root, mais n'a sinon aucune contrainte sur les cles. Le schema "natif" issu des variables relations non calculees (Table ou Entity) peut alors impliquer des dictionnaires ayant ou non des cles, a tous les niveau de la hierarchie. En fait, un schema multi-table est specifie selon sa structure hierarchique, native ou calculee. Un meme schema en flocon (ex: Customer-Services-Usages) peut etre ainsi alimente de plusieurs facon differentes: - depuis trois fichiers, si le schema est natif et exploite des dictionnaires avec des cles coherentes - depuis un seule table, avec un champs json permet de reconstruire le schema a la volee par une regle de construction de table, impliquant des dictionnaires ayant ou non des cles, coherentes enre elles ou non Dans la version V10 ou V11 de Khiops, un fichier de dictionnaire qui compile est necessairement utilisable pour specifier un mapping multi-tables, avec la liste des fichier necessaire a l'alimentattion des donnees Dans la version avec creation de table, les controles se font en deux temps - au moment du chargement du dictionnaire, on verifie tout ce qui est possible - pour un dictionnaire Root, on peut verifier que le systeme de cle de son schema est coherent - pour les schemas non Root, on ne peut pas verifier a priori la coherence des cle, car ces dictinnaires peuvent etre utilises en sortie de regles de creation de table - on memorise en indicateur interne les dictionnaire qui sont `storable`, ceux qui sont completement coherent vis a vis des cles - au moment du choix d'un dictionnaire d'analyse - on alimente la liste des fichiers selon le schema natif du dictionnaire principal, qu'il soit coherent ou non - cela permet ainsi de visualiser la structure native du schema - mais on ne genere pas en temps reel des messages d'erreur complexes, a chaque changement de choix de dictionnaire d'analyse - au moment du declenchement d'une action (apprentissage, analyse) - on genere des messages d'erreur, les plus didactiques possibles KWClass - IsKeyBasedStorable - Capacite a etre stocke sur un systeme de fichiers multi-tables a l'aide de cles - mise a jour par la methode IndexClass - CheckKeyBasedStorability: verification de la capacite a etre stocke, avec emission de message d'erreur - Check - appel des CheckNativeComposition avec verification des cles uniquement pour les classes Root - CheckNativeComposition(bCheckKeys, bVerboseCheckKeys) - test de non recursion dans la composition - test de validite des cles a la demande - InternalCheckNativeComposition: methode technique pour la gestion de la methode CheckNativeComposition - rappatriement de tous les controles sur la composition d'un dictionnaire et la coherence de ses cles, en provenance de KWClass::Check et KWAttribute::Check KWMTDatabase - CheckPartially - on verifie en prealable que le dictionnaire est stockable, avec si necessaire des messages d'erreur didactiques Tests systematiques dans LearningTest\TestKhiops\z_TableCreationRules
Pour des raisons de lisibilite du log, une ligne blanche est ajoute apres chaque action (apprentissage, deploiement..) Cela permet de separer les compte-rendus des actions et de les lire par blocs. On ajoute cette fois la ligne blanche apres le declenchement de l'action, que l'action soit executee ou non en raison d'erreurs de specification utilisateur. Cela permet d'isoler les messages d'erreur en blocs, pour empecher qu'ils soit "colles" a l'action suivante. Mise a jour des jeux de test CI/CD impactes (lignes blanches additionnelles dans le log) Tests complets sur LearningTest, avec mise a jour des jeux de test impactes
Avec la possibilité d'avoir des dictionnaire avec ou sans cle, on peut avoir des dictionnaires stockable ou non. Au moment du choix d'un dictionnaire d'analyse, dans la liste d'aide, on met en tete les dictionnaires storable, puis une ligne blanche, puis les dictionnaires non storable. On les garde tous dans la liste d'aide, pour des raisons didactiques, et pour ne pas "inquieter" les utilisateurs en cas d'absence de dictionnaires. Note: il aurait ete preferable de mettre ces nomde dictionnaire en gris dans les liste d'aide, mais le rapport cout/benefice de cette option est tres defavorable (necessite de modifier significativement le moteur de la GUI dans Norm) Impacts principaux - KWClassManagementActionView - mise en place d'une liste d'aide rafraichie dynamiquement, dediee aux noms dictionnaire, selon la specification - RefreshHelpLists: mise a jour de la liste, appelee dans EventRefresh - KWLearningProblemView::KWLearningProblemView: parametrage de la liste d'aide avec le service de KWClassManagementActionView - CCLearningProblemView::CCLearningProblemView: parametrage de la liste d'aide avec le service de KWClassManagementActionView - KWDatabaseTransferView::Open: variante similaire, mais avec liste d'aide locale (pas de rafaichssement necessaire) - KWDataTableKeyExtractorView::Open: commentaire expliquant pourquoi on garde cette fois tous les dictionnaires - KWDataTableSorterView::Open: commentaire expliquant pourquoi on garde cette fois tous les dictionnaires Impacts la bibliotheque java norm.jar - src/Norm/NormGUI/src/normGUI - GUIObject::getParametersAsArray: correction pour prendre en compte les items vides dans une liste de parametres - GUIStringElementHelpedComboBox: correction pour autoriser les doublons (deux fois la valeur vide, par exemple)
marcboulle
force-pushed
the
490-implement-coexistence-of-created-table-with-and-without-keys
branch
from
December 20, 2024 16:21
787b359
to
110b87f
Compare
popescu-v
approved these changes
Dec 20, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
marcboulle
deleted the
490-implement-coexistence-of-created-table-with-and-without-keys
branch
January 2, 2025 08:19
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Seuls les 4 derniers commits, propres à la PR, sont à prendre en compte