Skip to content

Fix Table.from_fixed reading data as schema for file-like schema_path - #813

Merged
jpmckinney merged 1 commit into
wireservice:masterfrom
Sanjays2402:fix/from-fixed-schema-file-object
Jul 4, 2026
Merged

jpmckinney merged 1 commit into
wireservice:masterfrom
Sanjays2402:fix/from-fixed-schema-file-object

Conversation

@Sanjays2402

Copy link
Copy Markdown
Contributor

What

Table.from_fixed() reads the data file as the schema whenever schema_path is passed as a file-like object instead of a path string, so it fails (or silently mis-parses).

Root cause

In agate/table/from_fixed.py, the schema file is resolved like this:

if not hasattr(schema_path, 'read'):
    schema_f = open(schema_path, encoding=schema_encoding)
    close_schema_f = True
else:
    schema_f = path          # <-- bug: `path` is the DATA file, not the schema

When schema_path already has a read attribute (a file-like object), the else branch assigns schema_f = path — the fixed-width data stream — rather than schema_f = schema_path. The data is then handed to the schema CSV reader.

The sibling branch a few lines above resolves the data file correctly (f = path in its own else), so only the schema branch is wrong. The existing test only passes string paths, which take the open(schema_path, ...) branch, so the file-object path was never exercised.

Symptom

import io, agate
schema = "column,start,length\nname,0,10\nage,10,3\n"
data   = "Alice     030\nBob       025\n"
agate.Table.from_fixed(io.StringIO(data), io.StringIO(schema))
ValueError: Schema must contain exactly three columns: "column", "start", and "length".

(And in the rarer case where the data's first line happens to parse as a valid schema, it silently produces the wrong columns rather than erroring.)

Fix

Assign schema_f = schema_path in the else branch, mirroring how the data file's else branch assigns f = path. One line.

case before after
schema_path = string path works works (unchanged)
schema_path = file-like object ValueError / wrong columns works

Test

Added test_from_fixed_file_like_objects, which passes the existing examples/testfixed and examples/testfixed_schema.csv fixtures as open file objects and asserts the resulting column names, types, and rows match the string-path result.

Proof it guards the bug (stash only the source fix, keep the test):

# with fix:            2 passed
# source fix stashed:  1 failed, 1 passed
#   FAILED test_from_fixed_file_like_objects
#   ValueError: Schema must contain exactly three columns ...
# fix restored:        2 passed

The pre-existing test_from_fixed (string-path branch) passes in every state, confirming the change is surgical.

Verification

  • Full suite: 396 passed (was 395; +1 new test)
  • ruff check on changed files: clean
  • isort --line-length 119 --check-only on changed files: clean

Also added a CHANGELOG.rst bullet under 1.14.3 - Unreleased and self-added to AUTHORS.rst.

When schema_path is passed as a file-like object (has a read attribute)
rather than a path string, from_fixed took the else branch and assigned
schema_f = path -- the data file object -- instead of schema_f =
schema_path. As a result the data stream was parsed as the schema, which
almost always raised "Schema must contain exactly three columns" (or, if
the data happened to look like a valid schema, silently produced the
wrong columns). Only the string-path branch, which reopens schema_path by
name, worked; the file-object branch was never exercised by the tests.

Assign schema_f = schema_path in the else branch, mirroring how the data
file's else branch assigns f = path.

Add a regression test that passes both the fixed-width file and its schema
as open file objects and asserts the parsed columns, types and rows match
the string-path result. It fails without the fix (ValueError from the
schema check) and passes with it.

@jpmckinney jpmckinney left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you! Looks like a clear typo.

@jpmckinney
jpmckinney merged commit f265d03 into wireservice:master Jul 4, 2026
21 checks passed
@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.48%. Comparing base (b58bb9c) to head (6293052).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #813      +/-   ##
==========================================
+ Coverage   93.41%   93.48%   +0.06%     
==========================================
  Files          98       98              
  Lines        2978     2978              
==========================================
+ Hits         2782     2784       +2     
+ Misses        196      194       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

2 participants