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

worksheet.set_column() keyword argument names don't match documentation #504

Closed
QJKX opened this issue Apr 9, 2018 · 3 comments
Closed
Assignees

Comments

@QJKX
Copy link

QJKX commented Apr 9, 2018

Hi!

firstcol, lastcol in worksheet.py
first_col, last_col in worksheet.html

@jmcnamara jmcnamara self-assigned this Apr 9, 2018
@jmcnamara
Copy link
Owner

Thanks for that.

Usually, in cases like this, I change the docs to reflect the code since there may be people suing the existing API. In this case I'd really prefer to change the API since first_col is used everywhere else.

Either way I'll fix it.

jmcnamara added a commit that referenced this issue Apr 10, 2018
Note, this is a backward incompatible change.

Closes issue #504
@jkyeung
Copy link
Contributor

jkyeung commented Apr 11, 2018

Not that you need my approval, but I totally agree with the decision to fix the API in this case, rather than adjust the docs.

My guess is that this is an especially benign change because most people probably specify those parameters positionally (i.e. not by keyword), which is how the examples do it. Honestly, I think that's probably why this inconsistency has gone unmentioned (and probably unnoticed!) for so long.

@jmcnamara
Copy link
Owner

@jkyeung thanks for the input on this. I changed the API and pushed the changes in version 1.0.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants