Skip to content

add: dragmode and template to layout#18

Merged
singjc merged 1 commit intoOpenMS:mainfrom
singjc:patch/peakmap_plotly
Aug 23, 2024
Merged

add: dragmode and template to layout#18
singjc merged 1 commit intoOpenMS:mainfrom
singjc:patch/peakmap_plotly

Conversation

@singjc
Copy link
Collaborator

@singjc singjc commented Aug 23, 2024

PR Type

enhancement


Description

  • Enhanced the Plotly figure layout by adding a template set to "simple_white".
  • Added dragmode set to "select" to improve user interaction with the plot.

Changes walkthrough 📝

Relevant files
Enhancement
core.py
Add dragmode and template to Plotly layout                             

pyopenms_viz/_plotly/core.py

  • Added template set to "simple_white" in the layout.
  • Added dragmode set to "select" in the layout.
  • +2/-0     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @singjc singjc merged commit 4de4312 into OpenMS:main Aug 23, 2024
    @qodo-code-review
    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No key issues to review

    @singjc singjc deleted the patch/peakmap_plotly branch August 23, 2024 01:37
    @qodo-code-review
    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Make layout parameters configurable instead of hardcoding them

    Consider making the template and dragmode parameters configurable instead of
    hardcoding them. This would provide more flexibility for users who might want to use
    different templates or drag modes.

    pyopenms_viz/_plotly/core.py [59-67]

     self.fig.update_layout(
         title=self.title,
         xaxis_title=self.xlabel,
         yaxis_title=self.ylabel,
         width=self.width,
         height=self.height,
    -    template="simple_white",
    -    dragmode="select"
    +    template=self.template,
    +    dragmode=self.dragmode
     )
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: This suggestion improves flexibility by allowing users to configure the template and dragmode parameters, which can be beneficial for different use cases. It addresses a potential limitation in the current implementation.

    8
    Best practice
    Add default values for new layout parameters with option to override

    Consider adding default values for template and dragmode in the class constructor or
    as class attributes. This would allow users to easily override these values if
    needed while maintaining the current behavior as default.

    pyopenms_viz/_plotly/core.py [59-67]

     self.fig.update_layout(
         title=self.title,
         xaxis_title=self.xlabel,
         yaxis_title=self.ylabel,
         width=self.width,
         height=self.height,
    -    template="simple_white",
    -    dragmode="select"
    +    template=self.template or "simple_white",
    +    dragmode=self.dragmode or "select"
     )
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: This suggestion provides a good balance between maintaining default behavior and allowing customization. It enhances usability by enabling users to override defaults while keeping the current settings as fallback options.

    7

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    1 participant