molgenis / molgenis-py-consensus

GNU Lesser General Public License v3.0
0 stars 3 forks source link

Fix/faster code #15

Closed marikaris closed 3 years ago

marikaris commented 3 years ago
SietsmaRJ commented 3 years ago

Globally: I see that a lot of string formatting has been replaced by F string formatting, this does force a user to use Python 3.7 or greater. Is this intentional?

I also see a lot of csv and tsv formatting in strings, which is not all that fast in Python. This isn't much of an issue if only ran twice each year, but upgrading to Pandas might be considered for even more speed. (If of course worth the investment.)

marikaris commented 3 years ago

Globally: I see that a lot of string formatting has been replaced by F string formatting, this does force a user to use Python 3.7 or greater. Is this intentional?

I also see a lot of csv and tsv formatting in strings, which is not all that fast in Python. This isn't much of an issue if only ran twice each year, but upgrading to Pandas might be considered for even more speed. (If of course worth the investment.)

Yes, the f-strings were intentionally because they are much faster than string formatting and they were easy to fix.

I agree that pandas would be great for the job and it would make the code a lot faster. I simply didn't have the time to refactor that much (and I don't have much experience with pandas, so it takes me a lot of time to get adjusted to again), but it's definitely something I want to do in the future.

sonarcloud[bot] commented 3 years ago

Kudos, SonarCloud 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