Skip to content

Dev madina#155

Merged
lfnothias merged 34 commits intodevfrom
dev_madina
Apr 30, 2025
Merged

Dev madina#155
lfnothias merged 34 commits intodevfrom
dev_madina

Conversation

@madina1203
Copy link
Collaborator

@madina1203 madina1203 commented Apr 26, 2025

PR Type

  • Enhancement
  • Documentation

Description

  • Updated SpectrumPlotter input parsing and error handling.

  • Improved database retrieval to check previous interactions.

  • Refactored Streamlit interface using dialogs and added video content.

  • Updated deployment scripts, configuration files, and license info.


Changes walkthrough 📝

Relevant files
Miscellaneous
1 files
tool_interpreter.py
Remove outdated commented API key setup code.                       
+0/-5     
Enhancement
6 files
tool_spectrum.py
Revamp input_data processing and nested JSON error handling.
+66/-40 
main.py
Clean up imports and streamline env variable loading.       
+22/-47 
tools_database.py
Enhance data retrieval to check previous interaction when data is
null.
+58/-10 
utils.py
Introduce IntRange class for argument range validation.   
+24/-0   
postgres_tool_database.py
Add initialize_db method for database connection management.
+3/-0     
streamlit_app.py
Replace modal usage with st.dialog and add illustrative videos.
+54/-44 
Documentation
2 files
questions.py
Adjust question list formatting with minor punctuation fix.
+2/-2     
README.md
Update environment activation command and change license to Apache
2.0.
+3/-3     
Configuration changes
3 files
setup.sh
Add setup script for Streamlit config and Heroku deployment.
+12/-0   
.python-version
Specify Python version 3.11 for the project.                         
+1/-0     
Procfile
Add Procfile to run the Streamlit app on Heroku.                 
+1/-0     

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @qodo-code-review
    Copy link

    Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Docstring Format

    The _run method in SpectrumPlotter now contains complex logic for parsing various input types but lacks a proper Google style docstring. Adding a detailed docstring with descriptions for all parameters and return values would improve clarity.

        self,
        input_data: Union[str, Dict[str, Any]] = None,
        usi: Optional[str] = None,
        run_manager: Optional[CallbackManagerForToolRun] = None,
    ) -> Dict[str, Any]:
        session_dir = create_user_session(self.session_id, user_session_dir=True)
        db_manager = tools_database()
        os.makedirs(session_dir, exist_ok=True)
        # If input_data is provided, it should be used instead of the other parameters
        if input_data:
            # If input_data is a string
            if isinstance(input_data, str):
                # Check if it's a CSV file path
                if input_data.endswith(".csv"):
                    df = pd.read_csv(input_data)
                    # Check required columns
                    if not {'usi'}.issubset(df.columns):
                        return {"error": "CSV file must contain 'usi' column."}
                    first_row = df.iloc[0]
                    usi = first_row['usi']
                else:
                    # Attempt to parse the string as JSON
                    try:
                        parsed = json.loads(input_data)
                    except Exception as e:
                        return {"error": f"Invalid input_data format: {e}"}
                    # If the parsed object contains '__arg1'
                    if isinstance(parsed, dict) and '__arg1' in parsed:
                        inner_val = parsed['__arg1']
                        # If the inner value is a string, try to parse it as JSON
                        if isinstance(inner_val, str):
                            try:
                                inner_json = json.loads(inner_val)
                            except Exception as e:
                                return {"error": f"Invalid nested JSON in '__arg1': {e}"}
                        elif isinstance(inner_val, dict):
                            inner_json = inner_val
                        else:
                            return {"error": "Unexpected type for '__arg1' value."}
                        if 'usi' in inner_json:
                            usi = inner_json['usi']
                        else:
                            return {"error": "Missing 'usi' key in nested JSON."}
                    # Otherwise, if the parsed JSON directly has 'usi'
                    elif 'usi' in parsed:
                        usi = parsed['usi']
                    else:
                        return {"error": "Invalid JSON structure. Must contain 'usi'."}
            # If input_data is already a dictionary
            elif isinstance(input_data, dict):
                if 'usi' in input_data:
                    usi = input_data['usi']
                elif '__arg1' in input_data:
                    inner_val = input_data['__arg1']
                    if isinstance(inner_val, str):
                        try:
                            inner_json = json.loads(inner_val)
                        except Exception as e:
                            return {"error": f"Invalid nested JSON in '__arg1': {e}"}
                    elif isinstance(inner_val, dict):
                        inner_json = inner_val
                    else:
                        return {"error": "Unexpected type for '__arg1' value."}
                    if 'usi' in inner_json:
                        usi = inner_json['usi']
                    else:
                        return {"error": "Missing 'usi' key in nested JSON."}
                else:
                    return {"error": "Invalid input. Provide a dictionary with 'usi' or '__arg1'."}
            else:
                return {
                    "error": "Invalid input type. Provide a dictionary with 'usi' or a valid CSV file path."
                }
    Missing Docstring

    The new IntRange class is useful but does not include a Google DocString format. Consider documenting its purpose, arguments, and methods in Google format for better maintainability.

    class IntRange:
        def __init__(self, start, end):
            self.start = start
            self.end = end
            self.range = range(start, end + 1)
    
        def __contains__(self, item):
            return item in self.range
    
        def __iter__(self):
            # for help display
            return iter([f"[{self.start}, {self.end}]"])
    
        def __str__(self):
            return f"[{self.start}, {self.end}]"
    
        def __repr__(self):
            return self.__str__()
    Function Documentation

    The new add_videos_to_content function improves UI functionality. To be consistent with documentation standards, add a Google docstring detailing its purpose, parameters (if any), and return value.

    def add_videos_to_content():
        st.markdown("---")
        st.subheader("Illustrative videos")
        st.markdown("Here are some illustrative videos to help you understand how to use MetaboT.")
        st.video("https://youtu.be/R6OiFt9ryxY")
        st.markdown("---")
        st.video("https://youtu.be/LpywYlHBWpw")

    @qodo-code-review
    Copy link

    qodo-code-review bot commented Apr 26, 2025

    Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Yield proper range integers

    Change the iterator to yield the full range of integers (instead of a single
    string) so that choices in argparse work correctly.

    app/core/utils.py [131-133]

     def __iter__(self):
    -    # for help display
    -    return iter([f"[{self.start}, {self.end}]"])
    +    return iter(self.range)
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    __

    Why: The improved code returns the actual range of integers instead of a string representation, which is a functional improvement for use cases like argparse that expect integer choices.

    Medium
    • Update

    @lfnothias lfnothias merged commit 253a7b9 into dev Apr 30, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    4 participants