jazzband / tablib

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

Refactor `import datetime` as `import datetime as dt` #569

Closed hugovk closed 8 months ago

hugovk commented 8 months ago

Follow on from https://github.com/jazzband/tablib/pull/568#discussion_r1375298382.

Refactor import datetime as import datetime as dt to avoid ambiguity between the datetime module and class:

codecov[bot] commented 8 months ago

Codecov Report

Merging #569 (a63cb1a) into master (c0e52cf) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #569   +/-   ##
=======================================
  Coverage   92.40%   92.40%           
=======================================
  Files          28       28           
  Lines        2938     2938           
=======================================
  Hits         2715     2715           
  Misses        223      223           
Files Coverage Δ
tests/test_tablib.py 98.84% <100.00%> (ø)
tests/test_tablib_dbfpy_packages_utils.py 100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

claudep commented 8 months ago

I'm a bit hesitant to touch dbfpy too much, as it is vendored code. I we were to update the vendored code to a more recent version, we would face a big diff. What do you think?

hugovk commented 8 months ago

Good point, I'll revert.

Shall we also rename the package directory to make it more obvious this is vendored?

For example, pip has _vendor:

claudep commented 8 months ago

Shall we also rename the package directory to make it more obvious this is vendored?

Makes sense, as long as this doesn't introduce compatibility issues (but it should not at first sight).

hugovk commented 8 months ago

Shall we also rename the package directory to make it more obvious this is vendored?

Makes sense, as long as this doesn't introduce compatibility issues (but it should not at first sight).

Hmm, I guess in theory someone could be doing from tablib.packages import dbfpy...

claudep commented 8 months ago

That could happen, but frankly I don't see the use case. I let you judge if moving the package is worth it, considering that minor breakage.

hugovk commented 8 months ago

Normally this sort of thing should be preceded by a deprecation warning, but I think we're fine here.

And the next release is set to be a major release anyway for https://github.com/jazzband/tablib/issues/558, so let's go for it.