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

Transliterate Greek according to ELOT 743. Untested #4

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

SStelioss
Copy link

@SStelioss SStelioss commented Jan 4, 2023

This is as far as I'm gonna go today, bit of a noob in python and I hope it doesn't show too much. Doesn't work properly, and I have no idea why, but I figure I'm pretty close? And at least it doesn't throw an error! Probably... It should be working. Anyway. Heavily WIP, consider merging after testing.

Current limitations

Can't do accents according to the standard, expects a de-accented input. Doesn't work. Possible redundancy in the ifs. Suggestions to improve welcome!

It does not sanity-check inputs or outputs, and has no testing it can run to make sure future edits don't break something.

@SStelioss
Copy link
Author

SStelioss commented Jan 4, 2023

Oh my god. Right after I did the pull request, I figured out why it doesn't work; I replace the thing just fine, but I never actually commit any of my changes to a variable...

EDIT: Nope, that's not it. The if statements after the for loops appear to not be called at all

@SStelioss
Copy link
Author

Works fine. I had three mistakes I fixed:

  1. Didn't properly declare my lists
  2. Couldn't handle ς, used a placeholder fix (replace all instances of ς with σ before processing, very placeholder)
  3. Hadn't actually comitted the string.replace to a variable

More testing is required, but I think it works fine.

@SStelioss SStelioss changed the title Transliterate Greek according to ELOT 743. WIP: Doesn't work properly. Transliterate Greek according to ELOT 743. Untested Jan 4, 2023
@SStelioss
Copy link
Author

Tested it a bit, seems to work fine. Possible point of failure is more than one word of input with two starting "Μπ"s (Μπαμπας Μπαμπουλας)

@sinnec sinnec linked an issue Jan 9, 2023 that may be closed by this pull request
Copy link
Owner

@sinnec sinnec left a comment

Choose a reason for hiding this comment

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

Good work. I would expect a few tests before merging. I could write the docs for that!

athenionn and others added 6 commits January 11, 2023 22:25
…t arose from that. Made lists into dictionary and simplified the "dumb" transliteration code thanks to that. Used remove_accentuation to allow for input of accented sentences. Added proper transliteration for "ου" in all cases accented or unaccented. To facilitate that, changed remove_accentuation.py to have an optional input that skips the diairesis step
…dded features as required for greek_elot_transliteration.py

In greek_elot_transliteration.py: Fixed bugs revolving around ου, made some mild readability improvements

In README.md: Minor grammar changes, documented changes to remove_accentuation.py as well as usage of greek_elot_transliteration.py

In test_tests.py: Added rudimentary tests. Not sure if they're properly implemented.
@SStelioss
Copy link
Author

SStelioss commented Jan 19, 2023

Good work. I would expect a few tests before merging. I could write the docs for that!

I think I did the tests? I wrote them, at the very least. I've never written tests before hahah. Would appreciate some feedback.

Speaking of docs, I wrote some! My branch conflicted with yours, and I had no idea how to properly merge 'em so I improvised a little. Def not a good way of doing things, but ah well.

I'm thinking it's in a state where it could be merged, but I'd appreciate your feedback on all the changes I made, and if there's any way I can improve it.

@sinnec
Copy link
Owner

sinnec commented Jan 22, 2023

Sorry for the delay but couldn't find time to review it. I'll test it and leave a comment at the code section and a suggestion! But great work altogether!

Comment on lines +89 to +120
# Accent based digraphs
# el_low_acc_digraphs = [
# "άυ",
# "αϋ",
#
# "έυ",
# "εϋ",
#
# "ήυ",
# "ηϋ"
#
# ]
# el_mix_acc_digraphs = [
# "Άυ",
# "Αϋ",
#
# "Έυ",
# "Εϋ",
#
# "Ήυ",
# "Ηϋ"
# ]
# el_cap_acc_digraphs = [
# "ΆΥ",
# "ΑΫ",
#
# "ΈΥ",
# "ΕΫ",
#
# "ΉΥ",
# "ΗΫ"
# ]
Copy link
Owner

Choose a reason for hiding this comment

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

I suggest we delete all commented code since it's not used!

Copy link
Author

Choose a reason for hiding this comment

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

Hmm... Reminder to myself not to use GitHub on the phone again. It had me fooled, thinking I only had one notification haha

Sure! I'll keep it saved in a file or something on my own computer for future use.

@@ -1,4 +1,4 @@
def remove_accentuation(string: str):
def remove_accentuation(string: str, modifier=0):
Copy link
Owner

@sinnec sinnec Jan 22, 2023

Choose a reason for hiding this comment

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

It should be modifier=True not 0. I also would like to rename modifier to something like keep_dieresis=True or something like that.

Comment on lines +27 to +32
if modifier == 0:
if c in dieresis.keys() and prev_char in ("ά", "ό", "έ"):
char = dieresis[c]
if modifier == 1: # Remove dieresis
if c in dieresis_reverse.keys():
char = dieresis_reverse[c]
Copy link
Owner

Choose a reason for hiding this comment

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

if modifier == 0 can become if modifier:which equals to if modifier == True,
and if modifier == 1 can become if not modifier: which equals to if modifier == False.

"φ",
"χ",
"ψ"
]
Copy link
Owner

Choose a reason for hiding this comment

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

I would preffer all list pairs tyo be converted into dicts but it's more of a readability enchancement!

@SStelioss
Copy link
Author

Sorry for the delay but couldn't find time to review it. I'll test it and leave a comment at the code section and a suggestion! But great work altogether!

No problem at all, and thanks! Take your time, there's no rush!

@sinnec
Copy link
Owner

sinnec commented Mar 20, 2023

Hey! Is there any progress?

@SStelioss
Copy link
Author

Hey! Sorry for not being active. I've been swamped with schoolwork, but once the easter break starts in a few weeks I plan to pick the project back up. Thanks for your understanding!

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.

Support for ELOT 743 Transliteration
2 participants