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

Wrong order of arguments in call to SplitTable and DelayedTable #1146

Open
KarstenSchulz opened this issue Apr 8, 2023 · 2 comments
Open

Comments

@KarstenSchulz
Copy link
Contributor

It seems, that calls to SplitTable and DelayedTable in genelements.py have a wrong order of arguments.

The definition of SplitTable is:

class SplitTable(DelayedTable):
    def __init__(self, data, colWidths, style, padding=3):
    	...

The call in genelements.py is:

SplitTable([['', rows]], style=t_style, colWidths=[0, None], padding=st.borderPadding,),

The order of style and colWidths are swapped.

The same observation can be made with DelayedTable. In line 1092 in the file genelements.py are the arguments style and colWidths swapped.

Description of problem

There is no observable problem, all tests pass.

🖥 Versions

python -V -> Python 3.10.9

pip freeze | grep rst2pdf -> I use the latest from github

pip freeze | grep reportlab -> reportlab==3.6.12

Which operating system are you using? MacOS 13.3.1

Additional information

If my observation is correct, I will search for other bugs of this kind and prepare a pull request. Please let me know.

@KarstenSchulz KarstenSchulz changed the title Wrong order of arguments in call to SplitTable and DelayTable Wrong order of arguments in call to SplitTable and DelayedTable Apr 8, 2023
@akrabat
Copy link
Member

akrabat commented Apr 10, 2023

As I understand keyword arguments, the order of the keyword arguments when calling a function does not matter. Having said that, consistency makes sense, so I'm okay with matching the order when we call with the defined order.

@KarstenSchulz
Copy link
Contributor Author

Yes, you are right, of course. However, if you want to learn and understand the source code of rst2pdf, as I do, it costs unnecessary mental power if the arguments are listed in a different order than the parameters. I am glad you agree and will finish a pull request in the next days. Thank you.

KarstenSchulz added a commit to KarstenSchulz/rst2pdf that referenced this issue Apr 11, 2023
This PR reorders the caller arguments according to the parameter list of the class to minimize mental debt when studying the code. No functional change. Tests pass.
akrabat added a commit that referenced this issue Apr 24, 2023
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

No branches or pull requests

2 participants