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

Add TARGWP20, TARGWP100 and TARGWP500 from third assessment report #28

Merged
merged 9 commits into from
Jun 14, 2024

Conversation

mikapfl
Copy link
Contributor

@mikapfl mikapfl commented Jun 11, 2024

We actually found a usage of global warming potentials from the third assessment report in the wild; therefore, we need them. Source is linked. I transferred it from pdf by hand (a mind-numbing exercise), so there might be errors. I'm willing to re-check tomorrow, but today it wouldn't make much sense.

@znicholls
Copy link
Contributor

I'll take a look now @mikapfl

Copy link
Contributor

@znicholls znicholls left a comment

Choose a reason for hiding this comment

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

Nice. A few typos and questions I think. Was indeed hard going through this with a pdf.

Is there a reason we're adding extra gases like "HGalden1040x"? Or is it just cause we've read them now, so may as well keep them?

globalwarmingpotentials.csv Outdated Show resolved Hide resolved
globalwarmingpotentials.csv Outdated Show resolved Hide resolved
globalwarmingpotentials.csv Outdated Show resolved Hide resolved
globalwarmingpotentials.csv Outdated Show resolved Hide resolved
globalwarmingpotentials.csv Show resolved Hide resolved
@mikapfl
Copy link
Contributor Author

mikapfl commented Jun 13, 2024

Note: I didn't include -(CF2)4CH(OH)- because the - are significant and we can't deal with them easily and it's just not worth the headache to figure something out given this is likely not used anywhere we care about.

@rgieseke
Copy link
Contributor

Note: I didn't include -(CF2)4CH(OH)- because the - are significant and we can't deal with them easily and it's just not worth the headache to figure something out given this is likely not used anywhere we care about.

Where is this not working? Should be a valid JS object or Python dictionary key, isn't it?

@mikapfl
Copy link
Contributor Author

mikapfl commented Jun 13, 2024

Where is this not working? Should be a valid JS object or Python dictionary key, isn't it?

It won't work as a normal unit in pint, i.e. in openscm-units, I think.

@mikapfl
Copy link
Contributor Author

mikapfl commented Jun 13, 2024

Thanks for checking everything, Zeb! Very much appreciated. To be extra sure, I now did an alternative extraction using https://smallpdf.com/pdf-to-excel and targeted copy+pasting, which means I didn't have the chance to introduce typos using this way. I found two missing values for HFE356pcc3 (I was apparently also confused, it is super confusing because the gases are not really ordered alphabetically or something).

Now I'm very confident that the values included here reflect what's in the official pdf.

@mikapfl
Copy link
Contributor Author

mikapfl commented Jun 13, 2024

Is there a reason we're adding extra gases like "HGalden1040x"? Or is it just cause we've read them now, so may as well keep them?

I just figured, if it is the pdf, let's add it now while we deal with this pdf and then never have to think about it again, hopefully.

@rgieseke
Copy link
Contributor

Where is this not working? Should be a valid JS object or Python dictionary key, isn't it?

It won't work as a normal unit in pint, i.e. in openscm-units, I think.

Maybe openscm-units should then just ignore it (it's very likely not handled by any SCM I guess). Not sure what level of completeness we have for other reports. If we include 'HGalden1040x' then why not '-(CF2)4CH(OH)-'?

@mikapfl
Copy link
Contributor Author

mikapfl commented Jun 13, 2024

Maybe openscm-units should then just ignore it (it's very likely not handled by any SCM I guess).

Ok, I added it and will deal with the fallout in openscm-units.

I did the same amount of checking for it (extract via two different routes) so am confident in the values as well.

@znicholls
Copy link
Contributor

Now I'm very confident that the values included here reflect what's in the official pdf.

Nice!

Ok, I added it and will deal with the fallout in openscm-units

Hmph, but fair I guess

Copy link
Contributor

@znicholls znicholls left a comment

Choose a reason for hiding this comment

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

Super, thank you!

README.md Show resolved Hide resolved
@rgieseke rgieseke merged commit 2a014b7 into openclimatedata:main Jun 14, 2024
@rgieseke
Copy link
Contributor

Released as 0.10.0, thanks @mikapfl and @znicholls!

@rgieseke
Copy link
Contributor

0.10.1 - forgot to re-run make.

@mikapfl
Copy link
Contributor Author

mikapfl commented Jun 14, 2024

0.10.1 - forgot to re-run make.

Something which could be included in pre-commit or other CI 😉

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

Successfully merging this pull request may close these issues.

3 participants