Skip to content
GitLab
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • O openpyxl
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 264
    • Issues 264
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 6
    • Merge requests 6
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Container Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • openpyxl
  • openpyxl
  • Merge requests
  • !414

Draft: Provide search function for worksheets

  • Review changes

  • Download
  • Email patches
  • Plain diff
Open jahrules requested to merge topic/3.1/Search into branch/3.1 May 15, 2022
  • Overview 76
  • Commits 15
  • Pipelines 13
  • Changes 6

--HG-- branch : 3.1

Rebased to 3.1 from 3.2

Outstanding Review comments (that I know of):

  • [1] search is still coercing to string. Haven't decided the best way to validate before search
  • [2] (DONE) still need to rebase the merge to version 3.1 iso 3.2
  • [3] (DONE) while the data structure is changed to a dict; I kept the return values as cells or arrays of cells. I did this for two reasons: it allowed me to validate the change to dictionary with minimal refactoring of the unit tests and two, I wanted Charlie's feedback on his view of the return types. Now that I know the internal structure change is working, changing the return type is relatively trivial.
  • [4] (DONE) based on review comments, I changed empty return types to 'None'. Some comments suggested throwing a warning or error. Personally, I prefer returning None and letting the user handle it; but am open to feedback.
  • [5] (DONE) while I did some updates to the test code based on the changes made, I still need to add some coverage. I opted to keep this for a separate merge.
  • [6] I still need to review general comments such as checking stack overflow a rereading the documentation from a 'new user' perspective so that I can make it more clear.
Edited May 21, 2022 by jahrules
Assignee
Assign to
Reviewers
Request review from
Time tracking
Source branch: topic/3.1/Search