Skip to content
This repository has been archived by the owner on Aug 10, 2023. It is now read-only.

Added the ability for users to use config files with V3 #1077

Merged
merged 16 commits into from
Mar 8, 2023

Conversation

EpicLiem
Copy link
Contributor

@EpicLiem EpicLiem commented Mar 7, 2023

I added the ability for people to add a config file using --config. There are some conflicts with a few new commits, so I still need to resolve them. Link this pr to issue #981.

@EpicLiem EpicLiem marked this pull request as ready for review March 7, 2023 19:08
@@ -22,7 +22,7 @@ class Chatbot:

def __init__(
self,
api_key: str,
api_key: str = None,
Copy link
Contributor Author

@EpicLiem EpicLiem Mar 7, 2023

Choose a reason for hiding this comment

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

Api key can be included in config, so we don't require it.

Copy link
Owner

@acheong08 acheong08 Mar 7, 2023

Choose a reason for hiding this comment

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

Need checks to see if it's included in at least 1 option?

Copy link
Contributor

@danparizher danparizher left a comment

Choose a reason for hiding this comment

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

Some formatting things to consider in comments

src/revChatGPT/V3.py Outdated Show resolved Hide resolved
src/revChatGPT/V3.py Outdated Show resolved Hide resolved
@joshuabrink
Copy link
Contributor

Wouldn't a more dry solution not require a config class variable and rather expand save and load functions, to work with config like:

    def save(self, file: str, *keys: str) -> None:
        """
        Save the Chatbot configuration to a JSON file
        """
        with open(file, "w", encoding="utf-8") as f:
            json.dump(
                {
                    key: self.__dict__[key]
                    for key in self.__get_filtered_class_keys(*keys)
                },
                f,
                indent=2,
            )

    def load(self, file: str, *keys: str) -> None:
        """
        Load the Chatbot configuration from a JSON file
        """
        with open(file, encoding="utf-8") as f:
            loaded_config = json.load(f)

            self.__dict__.update(
                {
                    key: loaded_config[key]
                    for key in self.__get_filtered_class_keys(*keys)
                }
            )

    def __get_filtered_class_keys(self, *keys: str) -> list:
        """
        Get filtered class keys from the given keys. 
        """
        class_keys = self.__dict__.keys() - {"session"}
        if not keys:
            return class_keys

        # Check if all passed keys are valid
        if invalid_keys := set(keys) - {"not"} - class_keys:
            raise ValueError(
                f"Invalid keys: {invalid_keys}",
            )

        # Remove the passed keys from the class keys.
        if keys[0] is "not":
            given_keys = keys[1:]
            return [key for key in class_keys if key not in given_keys]
        else:
            # Only return specified keys that are in class_keys
            return [key for key in keys if key in class_keys]

This would allow for commands like:

  • !load chat.json top_p temperature to only load top_p and temperature from a config file
  • !save chat.json not top_p temperature to save all fields except top_p and temperature to a config file
  • And simplify implementation with cli:
    if config:       
        chatbot = ChatbotCLI(args.api_key)
        try:
            chatbot.load(config)
        except:
            print(f"Error: {args.config} could not be loaded")
            sys.exit()
    else:
        chatbot = ChatbotCLI(
            api_key=args.api_key,
            system_prompt=args.base_prompt,
            proxy=args.proxy,
            temperature=args.temperature,
            top_p=args.top_p,
            reply_count=args.reply_count,
        )

EpicLiem and others added 2 commits March 7, 2023 14:43
Co-authored-by: Daniel Parizher <105245560+Arborym@users.noreply.github.com>
Co-authored-by: Daniel Parizher <105245560+Arborym@users.noreply.github.com>
@EpicLiem
Copy link
Contributor Author

EpicLiem commented Mar 7, 2023

Wouldn't a more dry solution not require a config class variable and rather expand save and load functions, to work with config like:

    def save(self, file: str, *keys: str) -> None:
        """
        Save the Chatbot configuration to a JSON file
        """
        with open(file, "w", encoding="utf-8") as f:
            json.dump(
                {
                    key: self.__dict__[key]
                    for key in self.__get_filtered_class_keys(*keys)
                },
                f,
                indent=2,
            )

    def load(self, file: str, *keys: str) -> None:
        """
        Load the Chatbot configuration from a JSON file
        """
        with open(file, encoding="utf-8") as f:
            loaded_config = json.load(f)

            self.__dict__.update(
                {
                    key: loaded_config[key]
                    for key in self.__get_filtered_class_keys(*keys)
                }
            )

    def __get_filtered_class_keys(self, *keys: str) -> list:
        """
        Get filtered class keys from the given keys. 
        """
        class_keys = self.__dict__.keys() - {"session"}
        if not keys:
            return class_keys

        # Check if all passed keys are valid
        if invalid_keys := set(keys) - {"not"} - class_keys:
            raise ValueError(
                f"Invalid keys: {invalid_keys}",
            )

        # Remove the passed keys from the class keys.
        if keys[0] is "not":
            given_keys = keys[1:]
            return [key for key in class_keys if key not in given_keys]
        else:
            # Only return specified keys that are in class_keys
            return [key for key in keys if key in class_keys]

This would allow for commands like:

* `!load chat.json top_p temperature` to only load `top_p` and `temperature` from a config file

* `!save chat.json not top_p temperature` to save all fields except `top_p` and `temperature` to a config file

* And simplify implementation with cli:
    if config:       
        chatbot = ChatbotCLI(args.api_key)
        try:
            chatbot.load(config)
        except:
            print(f"Error: {args.config} could not be loaded")
            sys.exit()
    else:
        chatbot = ChatbotCLI(
            api_key=args.api_key,
            system_prompt=args.base_prompt,
            proxy=args.proxy,
            temperature=args.temperature,
            top_p=args.top_p,
            reply_count=args.reply_count,
        )

I'll refactor my code to use this approach

@EpicLiem
Copy link
Contributor Author

EpicLiem commented Mar 7, 2023

Does anyone know why the PR isn't updating? I added a commit to the branch, and it appears there, but they're not appearing here.

@EpicLiem
Copy link
Contributor Author

EpicLiem commented Mar 7, 2023

It seems I needed to add another commit, and now it updated the pr.

@EpicLiem EpicLiem marked this pull request as draft March 7, 2023 23:35
if config.get("reply_count") is not None:
reply_count = config.get("reply_count")
if config.get("system_prompt") is not None:
system_prompt = config.get("system_prompt")```
Copy link
Contributor

Choose a reason for hiding this comment

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

Accidentally left a Markdown artifact in here 😅

Suggested change
system_prompt = config.get("system_prompt")```
system_prompt = config.get("system_prompt")

@acheong08
Copy link
Owner

Conflicts

@EpicLiem EpicLiem marked this pull request as ready for review March 8, 2023 00:10
src/revChatGPT/V3.py Outdated Show resolved Hide resolved
Copy link
Owner

@acheong08 acheong08 left a comment

Choose a reason for hiding this comment

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

.

@EpicLiem
Copy link
Contributor Author

EpicLiem commented Mar 8, 2023

@acheong08 The custom submit key breaks things if a submit key isn't given because it then tries to reference the key_binding var when it's never defined. Should I fix that in this pr?

@acheong08
Copy link
Owner

@acheong08 The custom submit key breaks things if a submit key isn't given because it then tries to reference the key_binding var when it's never defined. Should I fix that in this pr?

Would be appreciated

src/revChatGPT/V3.py Outdated Show resolved Hide resolved
EpicLiem and others added 2 commits March 7, 2023 19:25
Co-authored-by: Daniel Parizher <105245560+Arborym@users.noreply.github.com>
@acheong08 acheong08 merged commit fdef83f into acheong08:main Mar 8, 2023
@EpicLiem EpicLiem deleted the 981-V3Config branch March 8, 2023 00:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants