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

Correct order of arguments to a regex call in safe_xlsx_sheet_title #510

Merged
merged 2 commits into from
Nov 4, 2021

Conversation

marisn
Copy link
Contributor

@marisn marisn commented Nov 2, 2021

PR #490 had non-functional tests and thus an invalid order of arguments to re.sub was unnoticed.
This PR corrects the call, adds tests + applies same treatment to sheet names also when saving a databook.

Fixes #489
Related to #490

@codecov
Copy link

codecov bot commented Nov 2, 2021

Codecov Report

Merging #510 (268370b) into master (1ba0705) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #510      +/-   ##
==========================================
+ Coverage   90.80%   90.82%   +0.02%     
==========================================
  Files          28       28              
  Lines        2653     2660       +7     
==========================================
+ Hits         2409     2416       +7     
  Misses        244      244              
Impacted Files Coverage Δ
src/tablib/formats/_xlsx.py 97.00% <100.00%> (ø)
tests/test_tablib.py 98.61% <100.00%> (+0.01%) ⬆️

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 1ba0705...268370b. Read the comment docs.

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

A couple of suggestions

src/tablib/formats/_xlsx.py Outdated Show resolved Hide resolved
tests/test_tablib.py Outdated Show resolved Hide resolved
Copy link
Contributor

@claudep claudep left a comment

Choose a reason for hiding this comment

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

Thanks!

@claudep claudep merged commit 91beb35 into jazzband:master Nov 4, 2021
@casimir
Copy link

casimir commented Jan 27, 2022

Hi is there a bugfix release planned for this change? Maybe version 3.1.1? The current latest (3.1.0) breaks exporting dataset in XLSX because of the sheet name being crushed with -.

@hugovk
Copy link
Member

hugovk commented Jan 27, 2022

Hi, yes, I think it's time for a new release.

Notable changes since 3.1.0 are this, #508 (add support for Python 3.10) (edit: 3.10 was added in #504 and already released) and #513 (drop support for EOL Python 3.6):

v3.1.0...master

@claudep Do you think dropping 3.6 warrants a major bump (Tablib 4.0.0) or minor (Tablib 3.2.0)?

I generally go for major bumps when dropping Python versions, but I don't really mind as we're using python_requires so pip should handle things nicely.

@hugovk
Copy link
Member

hugovk commented Jan 27, 2022

(We called the 2.7 and 3.5 drops breaking changes and bumped to 1.0.0 and 3.0.0 respectively.)

@casimir
Copy link

casimir commented Jan 27, 2022

Just a little more context, I'm still stuck with python 3.6 for the few months to come. That's why I was talking about a bugfix release like 3.1.1 (I assumed semver without checking the project policy though).
On the other side I understand very well the burden of such process and I would also understand that you only cut a new release from the current tip.

@claudep
Copy link
Contributor

claudep commented Jan 27, 2022

@hugovk 3.2.0 would be fine for me.

@casimir In your specific situation, I would temporary install tablib from a specific git commit (pip supports that) until you can upgrade to Python 3.7+.

@casimir
Copy link

casimir commented Jan 27, 2022

I'll stick with the 3.0.0 while I'm stuck with Python 3.6. Just my 2 cents but I would at least yank the 3.1.0 on pypi as this is a really disruptive bug.

Thanks for the quick response. And also thanks for reviving this nifty project.

@hugovk
Copy link
Member

hugovk commented Jan 27, 2022

3.2.0 released!

https://pypi.org/project/tablib/3.2.0/


You can tell pip to ignore the Python version check with --ignore-requires-python and hope for the best:

$ python3.6 -m pip install tablib==3.2.0
ERROR: Could not find a version that satisfies the requirement tablib==3.2.0 (from versions: 0.6.1, 0.6.2, 0.6.3, 0.6.4, 0.7.0, 0.7.1, 0.8.0, 0.8.1, 0.8.2, 0.8.3, 0.8.4, 0.9.0, 0.9.1, 0.9.2, 0.9.3, 0.9.4, 0.9.5, 0.9.6, 0.9.7, 0.9.8, 0.9.9, 0.9.10, 0.9.11, 0.10.0, 0.11.0, 0.11.1, 0.11.2, 0.11.3, 0.11.4, 0.11.5, 0.12.0, 0.12.1, 0.13.0, 0.14.0, 1.0.0, 1.1.0, 2.0.0, 3.0.0, 3.1.0)
ERROR: No matching distribution found for tablib==3.2.0
$ python3.6 -m pip install tablib==3.2.0 --ignore-requires-python
Collecting tablib==3.2.0
  Using cached tablib-3.2.0-py3-none-any.whl (48 kB)
Installing collected packages: tablib
  Attempting uninstall: tablib
    Found existing installation: tablib 3.1.0
    Uninstalling tablib-3.1.0:
      Successfully uninstalled tablib-3.1.0
Successfully installed tablib-3.2.0
Python 3.6.12 (default, Oct 19 2020, 21:40:56)
[GCC 4.2.1 Compatible Apple LLVM 10.0.1 (clang-1001.0.46.4)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import tablib
>>> data = tablib.Dataset(headers=['First Name', 'Last Name', 'Age'])
>>> for i in [('Kenneth', 'Reitz', 22), ('Bessie', 'Monke', 21)]:
...     data.append(i)
...
>>> print(data.export('json'))
[{"First Name": "Kenneth", "Last Name": "Reitz", "Age": 22}, {"First Name": "Bessie", "Last Name": "Monke", "Age": 21}]
>>>

Or install from a specified git commit as mentioned.

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.

"Invalid character / found in sheet title" during export to XLSX
4 participants