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

Bugfix to preserve numeric values when using CSV based templates #182

Closed
wants to merge 1 commit into from

Conversation

syrenius
Copy link

@syrenius syrenius commented Jul 6, 2021

When reading the template fro a CSV file, all values in the elements dictionary end up as strings wich in turn causes problems with some other functions that rely on mathematical operators (e.g. the rgb(col) function). This is a quick fix to try to preserve numeric values.

@Lucas-C
Copy link
Member

Lucas-C commented Jul 7, 2021

Thank you for your contribution and welcome @syrenius ! 😊

Please, could you provide a unit test,
or just a piece of code demonstrating the need for this fix?

@alexp1917
Copy link
Collaborator

google suggests using pandas to parse csv data preserving the original data type, but i think thats overkill. I did figure out where the problem is, and i think we may want to instead of handling this as a general csv problem, handle it in the specific instances where the below can cause an issue:

https://github.com/PyFPDF/fpdf2/blob/3d4627e1d6f285b1976a99f04319c6258d57c5c1/fpdf/template.py#L185-L187

Copy link
Collaborator

@alexp1917 alexp1917 left a comment

Choose a reason for hiding this comment

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

^

@Lucas-C
Copy link
Member

Lucas-C commented Jul 21, 2021

I @syrenius !

I noticed you are a newcomer here on GitHub.

Feel free to ask all the questions you have regarding how things work:
this project (fpdf2), Pull Requests (PR) like this one, how to update them,
how to run tests or check the code with Pylint...

Regards 😉

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.

3 participants