scribe-org / Scribe-Data

Wikidata, Wiktionary and Wikipedia language data extraction
GNU General Public License v3.0
30 stars 69 forks source link

Update utils.py #63

Closed Jk40git closed 8 months ago

Jk40git commented 9 months ago

Change the _find's docstring to _load_json's

Contributor checklist


Description

Related issue

github-actions[bot] commented 9 months ago

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. It'd be great to have you!

Maintainer checklist

andrewtavis commented 9 months ago

I see there was some confusion here, @Jk40git. Big thing is that we don't want the docstring to "be the docstring of the other one", but we want them to have the same style. For find we currently have:

    """
    Each 'language', (english, german,..., etc) is a dictionary of key/value pairs:

        entry = {
            "language": "english",
            "iso": "en",
            "qid": "Q1860",
            "remove-words": [...],
            "ignore-words": [...]
        }

    Given a key/value pair, the 'source', and the 'target' key, get the 'target' value.

    Args:
        source_value (str): e.g. 'english'.
        source_key (str): e.g. 'language'.
        target_key (str): e.g. 'iso'.
        error_msg (str): for when a value cannot be found.

    Raises:
        ValueError: when a source_value is not supported.

    Returns:
        The 'target' value given the passed arguments.
    """

What this should be is:

    """
    Each 'language', (english, german,..., etc) is a dictionary of key/value pairs:

        entry = {
            "language": "english",
            "iso": "en",
            "qid": "Q1860",
            "remove-words": [...],
            "ignore-words": [...]
        }

    Given a key/value pair, the 'source', and the 'target' key, get the 'target' value.

    Parameters
    ----------
        source_value : str
            The source value to find equivalents for (e.g. 'english').
        source_key : str
            The source key to reference (e.g. 'language').
        target_key : str
            The key to target (e.g. 'iso').
        error_msg : str
            The message displayed when a value cannot be found.

    Raises
    ------
        ValueError : when a source_value is not supported.

    Returns
    -------
        The 'target' value given the passed arguments.
    """

The big thing is that everywhere that we have Args: in a docstring we want:

Parameters
----------
    {LIST_OF_THE_ARGS}

And everywhere we have Returns: we want:

Returns
-------
    {LIST_OF_RETURN_VALUES}

Can you convert the docstring of _find over to what I have above, and then I can send along a list of functions that we also need to do other conversions for?

Jk40git commented 9 months ago

@andrewtavis Just made an edit #63

andrewtavis commented 9 months ago

Nice, yes, this is looking good @Jk40git! Here are the other docstrings that need edits:

Want to send along another commit with all of these changes, and then we can close this up? In cases where there's a section that's not Args: or Returns:, let's just keep it in order and put dashes under it. So for example Raises: we can also do:

Raises
------

as in the example above :) Let me know if there are any questions on this!

Jk40git commented 9 months ago

Nice, yes, this is looking good @Jk40git! Here are the other docstrings that need edits:

  • In checkquery.py basically all for function docstrings need to be edited
  • In utils.py we need _find as you already did and then check_command_line_args and check_and_return_command_line_args

Want to send along another commit with all of these changes, and then we can close this up? In cases where there's a section that's not Args: or Returns:, let's just keep it in order and put dashes under it. So for example Raises: we can also do:

Raises
------

as in the example above :) Let me know if there are any questions on this!

Do you want me to change the check_command_line_args and check_and_return_command_line_args with the same docstring from _find?

andrewtavis commented 8 months ago

Don't worry about the Mac builds, by the way. This is known in #61 :)