jmcnamara / XlsxWriter

A Python module for creating Excel XLSX files.
https://xlsxwriter.readthedocs.io
BSD 2-Clause "Simplified" License
3.66k stars 631 forks source link

Add Argument Type Checks for Write.insert_image() #1027

Closed parisosuch-dev closed 1 year ago

parisosuch-dev commented 1 year ago

Example

# create workbook
workbook = xlsxwriter.Workbook(REPORT_PATH)
# create worksheet
worksheet = workbook.add_worksheet("Report")
# add image to worksheet
worksheet.insert_image(1, "A", FOOTER_PATH)

worksheet.autofit()

workbook.close()

This raises an error

File "C:\Users\...\.venv\lib\site-packages\xlsxwriter\worksheet.py", line 4477, in _check_dimensions
    if row < 0 or col < 0:
TypeError: '<' not supported between instances of 'str' and 'int'

Explanation

The reason for this addition is that the error trace for the argument type occurs in the _check_dimensions() comparison operand rather than in the parent function that calls it. Better error tracing and explanations make everyone happier!

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

jmcnamara commented 1 year ago

Thanks for the suggestion but this would need to be done collectively for all of the XlsxWriter APIs. That is planned for the future see #1028.

parisosuch-dev commented 1 year ago

Thanks for the suggestion but this would need to be done collectively for all of the XlsxWriter APIs. That is planned for the future see #1028.

Understandable. Going to go ahead and close this PR out. Would something such as PR that adds type hinting and checking for all of the files in the python module suffice? I would be more than happy to help make that happen.

jmcnamara commented 1 year ago

Would something such as PR that adds type hinting and checking for all of the files in the python module suffice? I would be more than happy to help make that happen.

Thanks for the offer but it is probably something that I need to tackle myself since there will be a lot of secondary refactoring to do at the same time.