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

[enhance] Process morphological mode in explore graph paths #86

Merged
merged 5 commits into from Jul 4, 2019

Conversation

Eradan94
Copy link
Collaborator

@Eradan94 Eradan94 commented Jun 26, 2019

Description

Process the morphological mode in explore graph paths.

Motivation and Context

Issue #80

How Has This Been Tested?

Tested with several graphs in French, Malagasy and Korean
Malagasy : V-tous.grf
French : see attachment and Dnum-ord.grf
Korean : tested with graphs given by Pr.Nam, but unsure about the jamo processing
tested-graphs.zip

Screenshots (if appropriate):

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
  • 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 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 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 Commit Message Guidelines
  • I understand that all commits on my pull request will be squashed to a single good one

@pullapprove pullapprove bot requested a review from martinec June 26, 2019 09:12
@Eradan94 Eradan94 changed the title Morphological mode Process morphological mode in explore graph paths Jun 26, 2019
Fst2List.cpp Outdated
@@ -269,6 +269,7 @@ class CFstApp {
char ofExt[16];
char ofnameOnly[512]; // output file name
char defaultIgnoreName[512]; // input file name
bool modeMorph = false; // true if the current state is in morphological mode
Copy link
Member

Choose a reason for hiding this comment

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

Have you noticed this suggestion #82 (comment) ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, i forgot this one. Done.

@pullapprove pullapprove bot requested a review from gvollant June 26, 2019 21:41
@martinec martinec closed this Jun 26, 2019
@martinec martinec requested review from martinec and removed request for gvollant June 26, 2019 21:48
@martinec martinec reopened this Jun 26, 2019
@martinec martinec changed the title Process morphological mode in explore graph paths [enhance] Process morphological mode in explore graph paths Jun 26, 2019
Copy link
Member

@martinec martinec left a comment

Choose a reason for hiding this comment

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

See #82 for a list of suggested changes

@pullapprove pullapprove bot requested a review from gvollant June 26, 2019 21:52
Fst2List.cpp Outdated
/**
* print one space when the morphological mode is off
**/
void outOneSpace() {
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I misunderstood your comment. What changes should I make?

Copy link
Member

Choose a reason for hiding this comment

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

What about appendSingleSpace instead of outOneSpace ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seems better than outOneSpace

@martinec martinec changed the base branch from master to feature/80 June 27, 2019 17:18
@martinec
Copy link
Member

martinec commented Jul 4, 2019

LGTM

@martinec martinec merged commit 1450e28 into UnitexGramLab:feature/80 Jul 4, 2019
@Eradan94 Eradan94 deleted the morphological_mode branch July 9, 2019 09:21
JM-ERI pushed a commit to JM-ERI/unitex-core that referenced this pull request Aug 16, 2020
martinec pushed a commit that referenced this pull request Sep 13, 2021
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.

None yet

2 participants