jazzband / tablib

Python Module for Tabular Datasets in XLS, CSV, JSON, YAML, &c.
https://tablib.readthedocs.io/
MIT License
4.59k stars 590 forks source link

Add default format detection for Databook.load() #488

Closed nuno-andre closed 3 years ago

nuno-andre commented 3 years ago

Unlike Dataset.load(), which defaults to None, Databook.load() needs an explicit format value.

This code raises TypeError: load() missing 1 required positional argument: 'format':

with open('data.xlsx', 'rb') as fh:
    dbook = Databook().load(fh)

This PR also (without api changes):

codecov[bot] commented 3 years ago

Codecov Report

Merging #488 (a875093) into master (7035d79) will increase coverage by 0.60%. The diff coverage is 89.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #488      +/-   ##
==========================================
+ Coverage   90.67%   91.28%   +0.60%     
==========================================
  Files          28       28              
  Lines        2616     2638      +22     
==========================================
+ Hits         2372     2408      +36     
+ Misses        244      230      -14     
Impacted Files Coverage Δ
src/tablib/core.py 87.05% <76.00%> (+3.08%) :arrow_up:
tests/test_tablib.py 98.61% <100.00%> (+0.05%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7035d79...a875093. Read the comment docs.

nuno-andre commented 3 years ago

Not the best PR, indeed. It's a bunch of minor changes prior to push some adapters we're using in prod.

So, in order to ease these and other contributions, would you consider setting those conventions and adding a code formatter as a pre-commit hook? Are there guidelines for this at Jazzband?

claudep commented 3 years ago

Frankly, I think it would be better to let formatting issues aside and concentrate on "real" changes instead, separated by concern.

nuno-andre commented 3 years ago

Code formatting reduces diffs and merging conflicts, and saves developers' time. In what sense is not a "real" change?

hugovk commented 3 years ago

Thanks for the PR!

Formatting is a real change, but please create another PR or issue for that (or see https://github.com/jazzband/tablib/issues/401 and linked PRs) and keep this one focused. If anything, it'll help us get this format detection reviewed and merged sooner.

Thank you!

nuno-andre commented 3 years ago

Right, I should have searched for such an issue... I'm going to close this PR (it's already too unfocused) and I'll open separate PRs.

Anyway, are there some guidelines for linting and code formatting at Jazzband?

hugovk commented 3 years ago

It varies by project. This one has some linting done by pre-commit, which you run using the pre-commit commands or with tox -e lint. The CI will check it too.