Implement CLI `--total` functionality #162

mhmohona · 2024-06-26T04:07:44Z

Contributor checklist

This pull request is on a separate branch and not the main branch

Description

Implemented the --total (-t) functionality would check Wikidata for the total of certain groupings of languages and word types.

Related issue

Fixes - #147

github-actions · 2024-06-26T04:08:00Z

Thank you for the pull request!

The Scribe team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)

If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Data rooms once you're in. Also consider joining our bi-weekly Saturday dev syncs. It'd be great to have you!

Maintainer checklist

The commit messages for the remote branch should be checked to make sure the contributor's email is set up correctly so that they receive credit for their contribution
- The contributor's name and icon in remote commits should be the same as what appears in the PR
- If there's a mismatch, the contributor needs to make sure that the email they use for GitHub matches what they have for git config user.email in their local Scribe-Data repo
The linting and formatting workflow within the PR checks do not indicate new errors in the files changed
The CHANGELOG has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)

mhmohona · 2024-06-26T04:25:28Z

It still has a problem. It is recognizing data-type as language like this -

andrewtavis · 2024-06-26T07:50:31Z

Any thoughts on what's causing it, @mhmohona? :)

mhmohona · 2024-06-29T14:47:06Z

src/scribe_data/cli/total.py

+}
+
+# Dictionary to map data types to their Wikidata Q-IDs
+data_type_to_qid = {


@andrewtavis will you please check here if I have the correct QID?

src/scribe_data/cli/total.py

andrewtavis · 2024-06-29T14:55:56Z

Minor comments so far:

Let's include the language and the data type in the output
From @wkyoshida, something like:

Language: German
Data type: Verbs
Total number of lexemes: 999

Let's move data_type_to_qid to the data_type_metadata.json file
Let's reference the language_metadata.json file for the language_to_qid information

We'll be good to go after all this! 🥳

andrewtavis · 2024-07-09T14:28:29Z

Quick check here @mhmohona, are you planning on getting to the changes we mentioned above?

wkyoshida · 2024-07-09T22:58:48Z

It still has a problem. It is recognizing data-type as language like this -

Doesn't this work since -l German is passed first? Could just be that the first argument is recognized and used. The second may have been passed, but it's disregarded.

Looks like there is a choices parameter that we can pass in to add_argument() to specify what are the allowable options for an argument. Maybe we look if specifying this could make sense as the mechanism for controlling valid/invalid inputs?

mhmohona · 2024-07-10T01:29:35Z

Currently stuck in this comment 😢

Let's move data_type_to_qid to the data_type_metadata.json file

andrewtavis · 2024-07-10T12:39:16Z

By this I mean let's move the functionality of data_type_to_qid to the data_type_metadata.json file such that we just import the data at the top of the file rather than using a custom function :)

mhmohona · 2024-07-10T18:48:23Z

It still has a problem. It is recognizing data-type as language like this -

Doesn't this work since -l German is passed first? Could just be that the first argument is recognized and used. The second may have been passed, but it's disregarded.

Looks like there is a choices parameter that we can pass in to add_argument() to specify what are the allowable options for an argument. Maybe we look if specifying this could make sense as the mechanism for controlling valid/invalid inputs?

@wkyoshida, thank you for looking into it. I have solved this problem.

mhmohona · 2024-07-10T18:51:04Z

By this I mean let's move the functionality of data_type_to_qid to the data_type_metadata.json file such that we just import the data at the top of the file rather than using a custom function :)

I need help with the QID. :( Unable to find the correct QIDs. It would be super helpful for me if you would update the data_type_metadata.json file with QIDs @andrewtavis.

mhmohona · 2024-07-11T09:30:26Z

I

Minor comments so far:

Let's include the language and the data type in the output

From @wkyoshida, something like:
Language: German
Data type: Verbs
Total number of lexemes: 999
Let's move data_type_to_qid to the data_type_metadata.json file

Let's reference the language_metadata.json file for the language_to_qid information

We'll be good to go after all this! 🥳

I have addressed 1st and 3rd feedback from here.

andrewtavis · 2024-07-25T03:55:14Z

src/scribe_data/cli/main.py

@@ -127,7 +128,11 @@ def main() -> None:
    )
    total_parser._actions[0].help = "Show this help message and exit."
    total_parser.add_argument(
-        "-lang", "--language", type=str, help="The language(s) to check totals for."
+        "-l",


I'm going to remove -l as an option, @mhmohona, as I think that making sure that we don't have commands and arguments that are the same is important :)

andrewtavis · 2024-07-25T03:56:53Z

src/scribe_data/cli/main.py

@@ -190,7 +195,13 @@ def main() -> None:
        )

    elif args.command in ["total", "t"]:
-        return
+        if not args.language and not args.data_type:


Really appreciate the quality of your messages to the user, @mhmohona!

andrewtavis · 2024-07-25T03:57:21Z

src/scribe_data/cli/main.py

+                "Error: At least one of -lang/--language or -dt/--data-type must be specified."
+            )
+            total_parser.print_help()
+            return


Let's always do a space after return :)

andrewtavis

Thanks so much for this, @mhmohona! The full experience of Scribe-Data is really coming along 😊

mhmohona added 5 commits June 25, 2024 17:49

add total functionality

ac63f68

fix formatting

0c14215

fix formatting

ac259d6

add docstring

43a01a8

fix formatting

beeba17

mhmohona changed the title ~~Add CLI --total functionality~~ Implement CLI --total functionality Jun 26, 2024

andrewtavis requested review from andrewtavis and wkyoshida June 26, 2024 07:49

mhmohona commented Jun 29, 2024

View reviewed changes

andrewtavis reviewed Jun 29, 2024

View reviewed changes

src/scribe_data/cli/total.py Show resolved Hide resolved

Merge branch 'main' into total

15c6d23

total cli error fix

e1629fe

mhmohona requested review from andrewtavis July 11, 2024 09:31

Merge branch 'main' into total

c449cab

andrewtavis reviewed Jul 25, 2024

View reviewed changes

Minor formatting of code to add spacing

a368d69

andrewtavis approved these changes Jul 25, 2024

View reviewed changes

andrewtavis merged commit 1bd659d into scribe-org:main Jul 25, 2024
2 checks passed

andrewtavis mentioned this pull request Jul 25, 2024

Implement CLI --total functionality #147

Closed

2 tasks

mhmohona deleted the total branch August 28, 2024 01:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement CLI `--total` functionality #162

Implement CLI `--total` functionality #162

mhmohona commented Jun 26, 2024

github-actions bot commented Jun 26, 2024 •

edited by andrewtavis

Loading

mhmohona commented Jun 26, 2024

andrewtavis commented Jun 26, 2024

mhmohona Jun 29, 2024

andrewtavis commented Jun 29, 2024

andrewtavis commented Jul 9, 2024

wkyoshida commented Jul 9, 2024

mhmohona commented Jul 10, 2024

andrewtavis commented Jul 10, 2024

mhmohona commented Jul 10, 2024

mhmohona commented Jul 10, 2024

mhmohona commented Jul 11, 2024

andrewtavis Jul 25, 2024

andrewtavis Jul 25, 2024

andrewtavis Jul 25, 2024

andrewtavis left a comment

Implement CLI --total functionality #162

Implement CLI --total functionality #162

Conversation

mhmohona commented Jun 26, 2024

Contributor checklist

Description

Related issue

github-actions bot commented Jun 26, 2024 • edited by andrewtavis Loading

Thank you for the pull request!

Maintainer checklist

mhmohona commented Jun 26, 2024

andrewtavis commented Jun 26, 2024

mhmohona Jun 29, 2024

Choose a reason for hiding this comment

andrewtavis commented Jun 29, 2024

andrewtavis commented Jul 9, 2024

wkyoshida commented Jul 9, 2024

mhmohona commented Jul 10, 2024

andrewtavis commented Jul 10, 2024

mhmohona commented Jul 10, 2024

mhmohona commented Jul 10, 2024

mhmohona commented Jul 11, 2024

andrewtavis Jul 25, 2024

Choose a reason for hiding this comment

andrewtavis Jul 25, 2024

Choose a reason for hiding this comment

andrewtavis Jul 25, 2024

Choose a reason for hiding this comment

andrewtavis left a comment

Choose a reason for hiding this comment

Implement CLI `--total` functionality #162

Implement CLI `--total` functionality #162

github-actions bot commented Jun 26, 2024 •

edited by andrewtavis

Loading