Skip to content

Read Excel columns by name and skip blank rows (fixes #136) - #138

Open
KevinBamwisho wants to merge 1 commit into
harmonydata:mainfrom
KevinBamwisho:fix/excel-flexible-columns
Open

Read Excel columns by name and skip blank rows (fixes #136)#138
KevinBamwisho wants to merge 1 commit into
harmonydata:mainfrom
KevinBamwisho:fix/excel-flexible-columns

Conversation

@KevinBamwisho

@KevinBamwisho KevinBamwisho commented Aug 20, 2026

Copy link
Copy Markdown

Description

The Excel parser read columns by position: column 0 as the question number, column 1 as the question, column 2 as the options, whatever the sheet actually held.

The spreadsheet attached to #136 is laid out Questionnaire | Question # | Question text | Notes, so on main it fails in two ways. The blank rows that separate one questionnaire from the next become questions with no text, and the schema rejects them:

pydantic_core._pydantic_core.ValidationError: 1 validation error for Question
question_text
  String should have at least 1 character [type=string_too_short, input_value='', input_type=str]

Delete those rows by hand and it parses, but everything is shifted one column over — the question number lands in question_text and the question itself lands in options:

question_no='The Fast Anxiety Instrument'  question_text='1'
options=['How often do you feel excessive worry or fear that feels difficult to control?']

With this change the same file, untouched, gives three instruments:

The Fast Anxiety Instrument   11 questions
GAD 7                          7 questions
PHQ-9                          9 questions

question_no='1'  question_text='Feeling nervous, anxious, or on edge'  instrument_name='GAD 7'

What it does:

  • Looks for a header row in the first five rows and reads the columns by name, so the question text column is found wherever it sits. Question, Question text, Item and Wording are all recognised, as are question number, options, questionnaire and notes columns. Patterns match the whole cell so that Question # is not mistaken for Question text.
  • Falls back to the old positional reading when no header is recognised, so sheets that parse today carry on parsing the same way. That code is unchanged, only moved into columns_by_position.
  • Drops blank rows up front, and skips any row with no question text instead of raising.
  • Uses a questionnaire column to split one sheet into several instruments. The name is carried down a block, so a name written once on the first row covers the rows below it, and it is set on the instrument and on its questions.
  • Keeps a notes column on the question in question_intro, which was previously hardcoded to "blah".

One decision worth your call: I put notes in question_intro because it is the only free text field on Question that survives matching. topics looked like the natural home but matcher.py overwrites it (all_questions[idx].topics = q_topics), so anything from the spreadsheet would be thrown away. Happy to move it if you would rather it went elsewhere.

No new dependencies, and one file changed plus one test file added.

Fixes #136

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Testing

New tests in tests/test_convert_excel_fluid_format.py build real spreadsheets and cover: the layout from the issue, blank rows between questionnaires, the question text and question number columns being found, several instruments in one sheet, unique instrument ids, the instrument name reaching the questions, notes being kept, the name carrying down a block, alternative column names (Item, Response options), junk rows above the header, a sheet with no header falling back to positional reading, an empty sheet, and a header with no rows under it.

Full test suite, run on this branch and on main for comparison:

passed skipped
main at 098182c 119 1
this branch 132 1

The difference is exactly the 13 new tests. No existing test changed status.

Lint: ruff check src tests passes.

Harmony API: I checked out harmonyapi, pointed its harmony submodule at this branch, started the API locally and ran tests/local_tests: 19 passed, 7 failed. I then repeated it with the submodule back on unmodified main and got the identical result, same 7 tests. Those 7 are the OpenAI, Azure OpenAI and Google Vertex cases, which need credentials I do not have, so they fail the same way with or without this change.

I also POSTed the spreadsheet from #136 to /text/parse on the running API and got HTTP 200 with the three instruments above, so the fix works through the API and not only in the library.

Two unrelated things I hit while setting that up, in case they are useful. Not touched in this PR:

  • harmonyapi/requirements.txt has no google-api-python-client, but wrapper_all_parsers.py imports google_forms_parser, which needs it. With the submodule on current main the API will not start until it is installed. The pinned submodule is older than Add Google Forms API integration for questionnaire import (closes #36) #126, which is why this is not showing up today.
  • model_downloader.py opens the models tarball with tarfile.open() and never closes it, then calls os.remove() on it. On Windows that raises PermissionError: [WinError 32]. Linux allows unlinking an open file, so CI never sees it.

Test Configuration

  • Library version: 1.0.7 (main at 098182c)
  • OS: Windows 11
  • Toolchain: Python 3.12.10 (CI uses 3.10.11), pandas 3.0.5, torch 2.2.2+cpu, pytest 9.1.1, ruff

Checklist

  • My PR is for one issue, rather than for multiple unrelated fixes.
  • My code follows the style guidelines of this project. I have applied a Linter (recommended: Pycharm's code formatter) to make my whitespace consistent with the rest of the project.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation — none needed, the public function keeps its signature and return type and the new helpers are internal
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings
  • The Harmony API is not broken by my change to the Harmony Python library
  • I add third party dependencies only when necessary. If I changed the requirements, it changes in requirements.txt, pyproject.toml and also in the requirements.txt in the API repo — none added here

The Excel parser took column 0 as the question number, column 1 as the
question and column 2 as the options, whatever the spreadsheet actually
held. A sheet laid out as Questionnaire | Question # | Question text |
Notes therefore ended up with the question number in question_text and
the question itself in options, and blank rows used to separate one
questionnaire from the next became questions with no text, which the
schema rejects with a ValidationError.

Look for a header row first and match the column names, falling back to
the old positional reading when no header is recognised. A questionnaire
column now splits a sheet into one instrument per questionnaire, and its
name reaches both the instrument and its questions. A notes column is
kept on the question, in place of the hardcoded "blah" intro.
@KevinBamwisho
KevinBamwisho force-pushed the fix/excel-flexible-columns branch from 2a3451e to 4dcbfd1 Compare August 20, 2026 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tolerate different formats of Excel

1 participant