Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix test to pass pytest -Werror #623

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

lbellomo
Copy link

@lbellomo lbellomo commented Mar 31, 2024

Refs #541

Still TODO:

  • Fix tests/test_cli.py::test_upsert_analyze
  • Fix tests/test_cli_memory.py::test_memory_csv
  • Fix tests/test_cli_memory.py::test_memory_tsv
  • Fix tests/test_cli_memory.py::test_memory_dump
  • Fix tests/test_cli_memory.py::test_memory_two_files_with_same_stem
  • Fix tests/test_insert_files.py::test_insert_files
  • Fix tests/test_recipes.py::test_dateparse_errors

📚 Documentation preview 📚: https://sqlite-utils--623.org.readthedocs.build/en/623/

@lbellomo
Copy link
Author

The first case is not the test you mark, but the previous test_upsert_pk_required. When you run the upsert "click" opens the file first but as it then gives the error "Error: Missing option '--pk" it doesn't close it correctly.

click.argument("file", type=click.File("rb"), required=True),

The solution is to pass lazy=True to the click.File so that it does not open it until it is necessary.

@lbellomo
Copy link
Author

It seems that in test_memory_csv only for when use_stdin=False the io.TextIOWrapper on line 297 is not closed correctly.

elif format == Format.CSV:
use_encoding: str = encoding or "utf-8-sig"
decoded_fp = io.TextIOWrapper(fp, encoding=use_encoding)
if dialect is not None:
reader = csv.DictReader(decoded_fp, dialect=dialect)
else:
reader = csv.DictReader(decoded_fp)
return _extra_key_strategy(reader, ignore_extras, extras_key), Format.CSV

The only way I can think of is to pass the buffer to _extra_key_strategy and close it once the generator is finished, but surely there must be a nicer way.

@lbellomo
Copy link
Author

Similar to the first case, it is not test_memory_dump but test_memory_csv_encoding the previous test that is the culprit, and also only when use_stdin=False.

It seems that when the enconding is wrong it raises a UnicodeDecodeError error when it tries to iterate over the generator from rows_from_file and that's why it doesn't reach the end to close the fp file.

fp = file_path.open("rb")
rows, format_used = rows_from_file(fp, format=format, encoding=encoding)
tracker = None
if format_used in (Format.CSV, Format.TSV) and not no_detect_types:
tracker = TypeTracker()
rows = tracker.wrap(rows)
if flatten:
rows = (_flatten(row) for row in rows)
db[file_table].insert_all(rows, alter=True)
if tracker is not None:
db[file_table].transform(types=tracker.types)
# Add convenient t / t1 / t2 views
view_names = ["t{}".format(i + 1)]
if i == 0:
view_names.append("t")
for view_name in view_names:
if not db[view_name].exists():
db.create_view(view_name, "select * from [{}]".format(file_table))
if fp:
fp.close()

The only thing that I can think of is to catch the error, close the file and raise it again.

@lbellomo
Copy link
Author

test_insert_files.py is a deprecation warning. datetime.utcfromtimestamp is replaced by datetime.fromtimestamp in py3.12, I pass the timezone to make sure it is utc (otherwise it grabs the local one) and stripe the timezone to be the same as it was before.

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.

1 participant