search

ProjectDrawdown / solutions

The mission of Project Drawdown is to help the world reach “Drawdown”— the point in the future when levels of greenhouse gases in the atmosphere stop climbing and start to steadily decline, thereby stopping catastrophic climate change — as quickly, safely, and equitably as possible.
https://www.drawdown.org/
Other
211 stars 91 forks source link

Rewrite of vma_xls_extract.py #510

Closed denised closed 2 years ago

denised commented 2 years ago

While extracting a solution I had an issue with VMAs, and found myself getting lost in the vma extraction code (not for the first time). So this is a rewrite, aimed at readability. It has almost exactly the same functionality as the original, with some simplifications. Its a little slower, because it does multiple passes over the data. On the plus side, it doesn't have any hardwired dependencies on tab names, rows or columns. But mostly, I believe it is a lot easier to read, so when issues arise it should be much easier to understand what is causing them.

I also revised some functionality in util.py, and created a single "turn a string into a filename" function that is shared between both extraction modules.

denised commented 2 years ago

@johnpaulalex For some reason I can't select you as a reviewer, but I would appreciate it if you would take a look at this code.

denised commented 2 years ago

Thank you @johnpaulalex for the very helpful review. I have included some lightweight pytest tests for the new code, but the more in-depth testing I did was a side-by-side comparison of the old and new implementations on all the recently imported solutions. (Which doesn't yet include any land models, so I will make a point of doing one of those soon.)

...And I just now spent a a few hours adding more testing of error/edge cases... which netted four bugs, so here's to testing :-)