search

nepalcodes / nepalingo

A website to learn the indigenous language of Nepal.
0 stars 0 forks source link

added eng to newari translation script #19

Closed NancyAanchal closed 1 week ago

NancyAanchal commented 1 week ago

I created a Python script using the nepal_bhase api that converts English words to Newari words while providing its meaning in both the langauges.

NancyAanchal commented 1 week ago

I wanted to extend my sincere appreciation for your valuable suggestions regarding the Python script. I have implemented all the changes you recommended, including raising an error with the response code and body when the API request fails.

In addition to your suggestions, I also added an exception for cases where the word's meaning is not found in the response.

I request you to review my new pull request and suggest any other possible improvements to it.

On Thu, Jun 27, 2024 at 7:52 AM binamkayastha @.***> wrote:

@.**** requested changes on this pull request.

Great work! I have some comments to improve the code a bit. No need to create a scripts folder in the scripts folder, just put it in the existing folder.

Note for future PRs: For every new feature/code try to create a branch name that matches the feature. So in this case you would name it something like "add-anki-flashcard-creation-script". This makes the git history cleaner (the generated merge conflict in cludes the branch name), and also allows you to have multiple branches open at the same time without resorting to "nancy1" and "nancy2" etc!

In scripts/scripts/EngToNewari.py https://github.com/nepalcodes/nepalingo/pull/19#discussion_r1655713633:

  • 'sec-ch-ua-mobile': '?0',
  • 'sec-ch-ua-platform': '"macOS"',
  • 'sec-fetch-dest': 'empty',
  • 'sec-fetch-mode': 'cors',
  • 'sec-fetch-site': 'cross-site',
  • 'sec-gpc': '1',
  • 'user-agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36'
  • }
  • response = requests.get(url, headers=headers)
  • if response.status_code == 200:
  • data = response.json()
  • return data
  • else:
  • return None

Instead of returning None and printing the error outside this function, let's raise an Error which includes the response code and body so we know why the error happened.

In scripts/scripts/EngToNewari.py https://github.com/nepalcodes/nepalingo/pull/19#discussion_r1655720474:

  • again=input('Go again?: (y/n) ')
  • if again=='n':
  • loop=False

One suggestion I have is to change this to a while True: loop, and then use the break keyword. This way you can get rid of the loop variable.

It's also generally best practice to assume the input is 'n' and only accept 'y' to continue the loop (Flip the comparison to be again != 'y') This way if someone accidentally types something random it doesn't continue doing something. (In this case it's not a big deal since you can always Ctrl+C out of the program, but let's try to follow best practices because it's a good habit)

— Reply to this email directly, view it on GitHub https://github.com/nepalcodes/nepalingo/pull/19#pullrequestreview-2143245052, or unsubscribe https://github.com/notifications/unsubscribe-auth/A7CF2LWD7SHVHDKMFV3DU4LZJNXVFAVCNFSM6AAAAABJ6MBHBCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNBTGI2DKMBVGI . You are receiving this because you authored the thread.Message ID: @.***>

binamkayastha commented 1 week ago

Thanks for the changes, looks great! No need to be so formal haha. In the future you can add commits to the same PR, no need to create a new one! For documentation purposes, new PR is: https://github.com/nepalcodes/nepalingo/pull/21

Closing this PR as a duplicate