Added function to write words as strings #661
Conversation
There was a problem hiding this comment.
@mariatsalakou this looks good, just a few hoops left to jump through and I'll be happy to merge.
| @@ -0,0 +1 @@ | |||
| DeclareOperation("WordToString", [IsString, IsList]); | |||
There was a problem hiding this comment.
This file requires the copyright header, same as the other files.
gap/fp/word.gi
Outdated
| function(alphabet, word) | ||
| local str, i, j; | ||
| str := ""; | ||
| j := PositionMaximum(word); |
There was a problem hiding this comment.
Why not just use j = Maximum(word);?
gap/fp/word.gi
Outdated
| [IsString, IsList], | ||
| function(alphabet, word) | ||
| local str, i, j; | ||
| str := ""; |
There was a problem hiding this comment.
Better to assign str to a value only after checking that alphabet and word are compatible.
gap/fp/word.gi
Outdated
| str := ""; | ||
| j := PositionMaximum(word); | ||
| if word[j] > Length(alphabet) then | ||
| return "Error, there are not enough letters in the alphabet"; |
There was a problem hiding this comment.
This isn't the correct way to do an error in GAP, you should use ErrorNoReturn, please look at another file in Semigroups to see an example.
gap/WordToString.g
Outdated
| @@ -0,0 +1,8 @@ | |||
| WordToString := function(alphabet, word) | |||
There was a problem hiding this comment.
You should remove this file completely, git rm gap/WordToString.g
There was a problem hiding this comment.
I think you forgot to remove this file @mariatsalakou
There was a problem hiding this comment.
Actually you re-introduced it when you did a merge.
configure.ac
Outdated
| @@ -23,7 +23,7 @@ AC_CANONICAL_HOST | |||
| dnl ## | |||
| dnl ## Setup automake | |||
| dnl ## | |||
| AM_INIT_AUTOMAKE([1.11 -Wall foreign subdir-objects]) | |||
There was a problem hiding this comment.
These changes are already in the master branch, it looks like you'll need to rebase your branch on the master branch: git fetch origin master && git rebase master (origin might have to be upstream or something else, depending on your setup.
gap/WordToString.g
Outdated
| @@ -0,0 +1,8 @@ | |||
| WordToString := function(alphabet, word) | |||
There was a problem hiding this comment.
I think you forgot to remove this file @mariatsalakou
gap/fp/word.gi
Outdated
| [IsString, IsList], | ||
| function(alphabet, word) | ||
| local str, i, j; | ||
| j := Maximum(word); |
There was a problem hiding this comment.
Not important, but you could just use Maximum(word) in the following line instead of defining j
gap/WordToString.g
Outdated
| @@ -0,0 +1,8 @@ | |||
| WordToString := function(alphabet, word) | |||
There was a problem hiding this comment.
Actually you re-introduced it when you did a merge.
gap/fp/random.gd
Outdated
| ############################################################################# | ||
| ## | ||
| ## random.gd | ||
| ## Copyright (C) 2020 James D. Mitchell |
There was a problem hiding this comment.
Given that you authored this, I think your name should be here as well!
gap/fp/random.gi
Outdated
| ## | ||
| ## random.gi | ||
| ## Copyright (C) 2020 James D. Mitchell | ||
| ## |
There was a problem hiding this comment.
This is very nearly ready to go. I made some minor edits, but there's one final thing that you'll have to do @mariatsalakou, and that's actually include the documentation you've written in the manual, which I suggest doing by adding a new section to the file docs/z-chap10.xml. I'm not 100% sure that the documentation you've written will compile (the RandomWord entry is missing the \<Description\> tags.
mariatsalakou
force-pushed
the
wordstostrings
branch
from
0621472 to
845a0dc
Compare
Co-authored-by: Murray Whyte <[email protected]>
|
This PR's superseded by #686. |
No description provided.