Skip to content
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

496 extend comment syntax in khiops dictionaries #503

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions src/Learning/KWData/KWAttribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ KWAttribute* KWAttribute::Clone() const
kwaClone->usName = usName;
kwaClone->metaData.CopyFrom(&metaData);
kwaClone->usLabel = usLabel;
kwaClone->svComments.CopyFrom(&svComments);
kwaClone->cType = cType;
kwaClone->attributeClass = attributeClass;
kwaClone->usStructureName = usStructureName;
Expand Down Expand Up @@ -205,6 +206,10 @@ boolean KWAttribute::Check() const
if (not KWClass::CheckLabel(GetLabel(), KWClass::Attribute, this))
bOk = false;

// Commentaires
if (not KWClass::CheckComments(GetComments(), KWClass::Attribute, this))
bOk = false;

// Type
if (not KWType::Check(GetType()))
{
Expand Down Expand Up @@ -445,6 +450,7 @@ longint KWAttribute::GetUsedMemory() const
lUsedMemory += usName.GetUsedMemory();
lUsedMemory += usName.GetValue().GetUsedMemory();
lUsedMemory += usLabel.GetUsedMemory();
lUsedMemory += svComments.GetUsedMemory();
lUsedMemory += metaData.GetUsedMemory() - sizeof(KWMetaData);

// Prise en compte de la regle de derivation
Expand Down Expand Up @@ -480,6 +486,12 @@ longint KWAttribute::ComputeHashValue() const

void KWAttribute::Write(ostream& ost) const
{
int i;

// Commentaires
for (i = 0; i < GetComments()->GetSize(); i++)
ost << "\t// " << GetComments()->GetAt(i) << "\n";

// Attribute a ne pas utiliser
if (not GetUsed())
ost << "Unused";
Expand Down Expand Up @@ -532,22 +544,32 @@ void KWAttribute::Write(ostream& ost) const
}
ost << "\t";

// Commentaire
// Libelle
if (GetLabel() != "")
ost << "// " << GetLabel();
}

void KWAttribute::WriteJSONFields(JSONFile* fJSON)
{
ALString sOutputString;
int i;

// Nom
fJSON->WriteKeyString("name", GetName());

// Commentaire
// Libelle
if (GetLabel() != "")
fJSON->WriteKeyString("label", GetLabel());

// Commentaires
if (GetComments()->GetSize() > 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder whether it wouldn't be more consistent to keep the contents of the if block in all cases, that is, have comments array even when there is no comment (i.e. GetComments()->GetSize() == 0 - in this case the array would just be empty, because the for loop would not have what to iterate on).

Copy link
Collaborator Author

@marcboulle marcboulle Jan 2, 2025

Choose a reason for hiding this comment

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

Je comprend la suggestion d'avoir un tableau vide plutôt que pas de tableau.
Mais l'absence de tableau me semble préférable pour les raisons suivantes:

  • conformités à la gestion des autres attributs facultatifs du json (libellé, meta-données...), qui ne sont écrits que s'ils sont non vides
  • compatibilité ascendante assurée avec les json V10 qui n'avaient pas de commentaires
  • parcimonie de stockage, en évitant potentiellement des dizaines de milliers de tableaux vides dans les json, pour les gros dictionnaires

{
fJSON->BeginKeyArray("comments");
for (i = 0; i < GetComments()->GetSize(); i++)
fJSON->WriteString(GetComments()->GetAt(i));
fJSON->EndArray();
}

// Attribute a ne pas utiliser
if (not GetUsed())
fJSON->WriteKeyBoolean("used", false);
Expand Down
17 changes: 17 additions & 0 deletions src/Learning/KWData/KWAttribute.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,15 @@ class KWAttribute : public KWDataItem
const KWTimestampTZFormat* GetTimestampTZFormat() const;

// Libelle
// Fin de ligne prefixee par '//' suivant la declaration de l'attribut dans le fichier dictionnaire
const ALString& GetLabel() const;
void SetLabel(const ALString& sValue);

// Commentaires
// Lignes prefixees par '//' precedent la declaration de l'attribut dans le fichier dictionnaire
const StringVector* GetComments() const;
void SetComments(const StringVector* svValue);

// Attribut effectivement utilise, chargeable en memoire
// Un attribut non utilise devient non charge
// Par defaut: true
Expand Down Expand Up @@ -241,6 +247,7 @@ class KWAttribute : public KWDataItem
// Specifications de l'attribut
KWCDUniqueString usName;
KWCDUniqueString usLabel;
StringVector svComments;
KWMetaData metaData;
KWClass* attributeClass;
KWCDUniqueString usStructureName;
Expand Down Expand Up @@ -401,6 +408,16 @@ inline void KWAttribute::SetLabel(const ALString& sValue)
usLabel.SetValue(sValue);
}

inline const StringVector* KWAttribute::GetComments() const
{
return &svComments;
}

inline void KWAttribute::SetComments(const StringVector* svValue)
{
svComments.CopyFrom(svValue);
}

inline boolean KWAttribute::GetUsed() const
{
return bUsed;
Expand Down
101 changes: 85 additions & 16 deletions src/Learning/KWData/KWAttributeBlock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,18 @@ boolean KWAttributeBlock::Check() const
if (not KWClass::CheckName(GetName(), KWClass::AttributeBlock, this))
bOk = false;

// Verification du Label
if (not KWClass::CheckLabel(GetLabel(), KWClass::AttributeBlock, this))
bOk = false;

// Verification des commentaires
if (not KWClass::CheckComments(GetComments(), KWClass::AttributeBlock, this))
bOk = false;

// Verification des commentaires internes
if (not KWClass::CheckComments(GetInternalComments(), KWClass::AttributeBlock, this))
bOk = false;

// Tests de base sur la specification du block
if (bOk and firstAttribute == NULL)
{
Expand Down Expand Up @@ -746,6 +758,8 @@ longint KWAttributeBlock::GetUsedMemory() const
lUsedMemory = sizeof(KWAttributeBlock);
lUsedMemory += usName.GetUsedMemory();
lUsedMemory += usLabel.GetUsedMemory();
lUsedMemory += svComments.GetUsedMemory();
lUsedMemory += svInternalComments.GetUsedMemory();
lUsedMemory += metaData.GetUsedMemory() - sizeof(KWMetaData);

// Prise en compte de la regle de derivation
Expand Down Expand Up @@ -778,42 +792,88 @@ longint KWAttributeBlock::ComputeHashValue() const

void KWAttributeBlock::Write(ostream& ost) const
{
KWAttribute* secondAttribute;
KWClass* parentClass;
KWAttribute* attribute;
int i;

ost << '{';
if (firstAttribute != NULL)
// Commentaires prededents le debut du bloc
Copy link
Collaborator

@popescu-v popescu-v Dec 20, 2024

Choose a reason for hiding this comment

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

s/prededents/precedant/ (present participle).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fait

for (i = 0; i < GetComments()->GetSize(); i++)
ost << "\t// " << GetComments()->GetAt(i) << "\n";
ost << "\t{\n";

// Attributs du bloc
parentClass = GetParentClass();
attribute = firstAttribute;
while (attribute != NULL)
{
ost << firstAttribute->GetName();
// Ecriture de l'attribut
ost << *attribute << "\n";

// Arret si derniere variable du bloc trouvee
if (attribute == lastAttribute)
break;

// Passage a l'attribut suivant
parentClass->GetNextAttribute(attribute);
}

// Acces au deuxieme attribut
secondAttribute = firstAttribute;
if (GetParentClass() != NULL)
GetParentClass()->GetNextAttribute(secondAttribute);
// Commentaires internes precedents la fin du bloc
for (i = 0; i < GetInternalComments()->GetSize(); i++)
ost << "\t// " << GetInternalComments()->GetAt(i) << "\n";
ost << "\t}";

// Si au moins trois attributs
if (lastAttribute != NULL and secondAttribute != NULL and lastAttribute != secondAttribute)
ost << ",..., ";
// Nom du bloc
ost << "\t" << KWClass::GetExternalName(GetName());
ost << "\t";

// Dernier attribut si au moins deux attributs
if (lastAttribute != NULL and lastAttribute != firstAttribute)
ost << ", " + lastAttribute->GetName();
// Regle de derivation
if (GetDerivationRule() != NULL)
{
// Dans le cas de la regle predefinie de Reference, on n'utilise pas le signe '='
if (GetDerivationRule()->GetName() != KWDerivationRule::GetReferenceRuleName())
ost << " = ";
GetDerivationRule()->WriteUsedRule(ost);
}
ost << "}\t" << GetName() << endl;

// Fin de declaration
ost << "\t;";

// Meta-donnees
if (GetConstMetaData()->GetKeyNumber() > 0)
{
ost << ' ';
GetConstMetaData()->Write(ost);
}
ost << "\t";

// Libelle
if (GetLabel() != "")
ost << "// " << GetLabel();
}

void KWAttributeBlock::WriteJSONFields(JSONFile* fJSON)
{
KWClass* parentClass;
KWAttribute* attribute;
ALString sOutputString;
int i;

// Nom
fJSON->WriteKeyString("blockName", GetName());

// Commentaire
// Libelle
if (GetLabel() != "")
fJSON->WriteKeyString("label", GetLabel());

// Commentaire
if (GetComments()->GetSize() > 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would discard the test and write the comments array anyway (even if empty).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cf. réponse plus haut
Mais l'absence de tableau me semble préférable pour les raisons suivantes:

  • conformités à la gestion des autres attributs facultatifs du json (libellé, meta-données...), qui ne sont écrits que s'ils sont non vides
  • compatibilité ascendante assurée avec les json V10 qui n'avaient pas de commentaires
  • parcimonie de stockage, en évitant potentiellement des dizaines de milliers de tableaux vides dans les json, pour les gros dictionnaires

{
fJSON->BeginKeyArray("comments");
for (i = 0; i < GetComments()->GetSize(); i++)
fJSON->WriteString(GetComments()->GetAt(i));
fJSON->EndArray();
}

// Regle de derivation
if (kwdrRule != NULL)
{
Expand Down Expand Up @@ -842,6 +902,15 @@ void KWAttributeBlock::WriteJSONFields(JSONFile* fJSON)
parentClass->GetNextAttribute(attribute);
}
fJSON->EndArray();

// Commentaires internes
if (GetInternalComments()->GetSize() > 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Idem, I would write the internalComments array anyway, even if empty. If the if is kept, then the BeginKeyArray and EndArray could be added outside the if block?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cf. réponse ci-dessus

{
fJSON->BeginKeyArray("internalComments");
for (i = 0; i < GetInternalComments()->GetSize(); i++)
fJSON->WriteString(GetInternalComments()->GetAt(i));
fJSON->EndArray();
}
}

const ALString KWAttributeBlock::GetClassLabel() const
Expand Down
33 changes: 33 additions & 0 deletions src/Learning/KWData/KWAttributeBlock.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,20 @@ class KWAttributeBlock : public KWDataItem
void ImportMetaDataFrom(const KWAttributeBlock* sourceAttributeBlock);

// Libelle
// Fin de ligne prefixee par '//' suivant la declaration du bloc d'attributs dans le fichier dictionnaire
const ALString& GetLabel() const;
void SetLabel(const ALString& sValue);

// Commentaires
// Ensemble des lignes prefixees par '//' precedant le debut du bloc d'attributs '{' dans le fichier dictionnaire
const StringVector* GetComments() const;
void SetComments(const StringVector* svValue);

// Commentaires internes
// Ensemble des lignes prefixees par '//' precedant la fin du bloc d'attributs '}' dans le fichier dictionnaire
const StringVector* GetInternalComments() const;
void SetInternalComments(const StringVector* svValue);

/////////////////////////////////////////////////////////
// Informations de specification du bloc, suite a sa
// creation depuis la classe englobante
Expand Down Expand Up @@ -259,6 +270,8 @@ class KWAttributeBlock : public KWDataItem
// Specifications du bloc
KWCDUniqueString usName;
KWCDUniqueString usLabel;
StringVector svComments;
StringVector svInternalComments;
KWDerivationRule* kwdrRule;
KWMetaData metaData;

Expand Down Expand Up @@ -373,6 +386,26 @@ inline void KWAttributeBlock::SetLabel(const ALString& sValue)
usLabel.SetValue(sValue);
}

inline const StringVector* KWAttributeBlock::GetComments() const
{
return &svComments;
}

inline void KWAttributeBlock::SetComments(const StringVector* svValue)
{
svComments.CopyFrom(svValue);
}

inline const StringVector* KWAttributeBlock::GetInternalComments() const
{
return &svInternalComments;
}

inline void KWAttributeBlock::SetInternalComments(const StringVector* svValue)
{
svInternalComments.CopyFrom(svValue);
}

inline KWAttribute* KWAttributeBlock::GetFirstAttribute() const
{
return firstAttribute;
Expand Down
Loading
Loading