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

[feature/multi2delaf] Add the transcoding function #125

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

selgueti
Copy link
Contributor

@selgueti selgueti commented Jul 15, 2022

Description

Add a function to transcode the multidelaf string into a delaf-format tag.

Motivation and Context

[issue #51] Generate dictionaries based on morphological dictionary-graphs

specif-multi-to-delaf.pdf

Type of files

  • bin: Binary files
  • ci: Continuous integration files
  • doc: Documentation files
  • Plain-text source code files

Level of change

  • break: Breaking change
  • exp: Experimental change
  • tmp: Temporal change
  • major: Major change
  • minor: Minor change
  • revert: Revert a commit change
  • sec: Vulnerability-related change
  • None of the above (normal change)

Type of change

  • deprecat: Deprecation of a once-stable feature
  • enhance: Enhancement in existing functionality
  • fix: Bug fix
  • feature: New feature
  • hotfix: Hotfix for bugs
  • refactor: Improve coding style, comments
  • remove: Remove a feature

Checklist:

  • My code compiles
  • My code does not generate new warnings
  • My code only implements a single fix or feature
  • My code follows the code style of this project
  • My code includes javadoc/doxygen where appropriate
  • My code is well factored, so that there is not repetitive code in the wild
  • My code does not refactor the surrounding code unless necessary
  • My code does not require a change in the documentation, if so I already opened an issue to list the changes
  • I have read the CONTRIBUTING document
  • I have read the Pull Request/Commit Message Guidelines
  • I have given a clear and concise title to my pull request following the above guidelines
  • I understand that all commits on my pull request will be squashed to a single good one
  • All above points were checked and are marked

@pullapprove pullapprove bot requested a review from martinec July 15, 2022 17:10
Multi2Delaf(const char* config_filename);
~Multi2Delaf();
// Not copyable or movable
Multi2Delaf(const Multi2Delaf&) UNITEX_EQ_DELETE;
Copy link
Member

Choose a reason for hiding this comment

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

As suggestion, if you use:

UNITEX_DISALLOW_COPY_AND_ASSIGN(Multi2Delaf);

This will expand to:

Multi2Delaf(const Multi2Delaf&)     UNITEX_EQ_DELETE;
void operator=(const Multi2Delaf&)  UNITEX_EQ_DELETE;

src/Fst2List.cpp Outdated
{NULL,no_argument_TS,NULL,0}
};

int main_Fst2List(int argc, char* const argv[]) {
char* ofilename = NULL;
char morpho_dic[1025] = "";
char *config_file_name = NULL;
Copy link
Member

Choose a reason for hiding this comment

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

char* config_file_name = NULL;
instead of
char *config_file_name = NULL;

src/Fst2List.cpp Outdated
@@ -295,6 +299,10 @@ class CFstApp {
bool isMdg; // true if the graph is a morphological dictionary-graph
struct hash_table* path_to_stop; /* a hash table to know all the Fst2Tag whose path exploration must be interrupted */
struct hash_table* dela_entries; /* a hash table to get the dela_entries of created boxes when lexical masks are processed */
bool compileToDelaf = false;
Multi2Delaf *multi2Delaf = NULL;
Copy link
Member

Choose a reason for hiding this comment

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

Multi2Delaf* multi2Delaf = NULL;

@martinec
Copy link
Member

Please also check for compilation errors on Travis : https://app.travis-ci.com/github/UnitexGramLab/unitex-core/builds/254307384#L990

@martinec
Copy link
Member

As appveyor shows some linking errors (https://ci.appveyor.com/project/martinec/unitex-core/builds/44442271#L1729), in order to compile with VS, if you introduce new .cpp files, first check if these files are referenced on src/build/Unitex4_vs2019.*

@martinec
Copy link
Member

I will continue the review once the code can be built without issues. Thank you for your commitment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants