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

Shale reference compositions and McLennan 2001 crustal compositions #37

Merged
merged 13 commits into from
Jul 7, 2020

Conversation

kaarelmand
Copy link

@kaarelmand kaarelmand commented Mar 3, 2020

Some new, mostly shale-based reference compositions for pyrolite.

Description

This PR adds some composition .csv files to the refcomp folder, mainly those concerning shales and other sediments.

  • Phanerozoic cratonic shale compilation by Condie (1993) -- PHS_Condie1993.csv
  • Post-Archean Australian Shale (PAAS) by Taylor & McLennan (1985), updates by McLennan (2001) -- PAAS_TaylorMcLennan1985.csv
  • North American Composite Shale (NASC) by Gromet et al. (1984), updates by Taylor & McLennan (1985), Condie, 1993, McLennan, 2001 -- NASC_Gromet1984.csv
  • Upper continental crust, lower continental crust and bulk continental crust composition estimates by McLennan (2001) -- UCC_McLennan2001.csv, LCC_McLennan2001.csv, BCC_McLennan2001.csv
  • Proterozoic and Archean cratonic shales by Condie (1993) -- PRS_Condie1993.csv, ARS_Condie1993.csv
  • "PAAS," being a smoother REY dataset from a different set of Australian shales by Pourmand et al. (2012) -- PAAS_Pourmand2012.csv
  • European Shale (EUS) by Bau et al. (2018) -- EUS_Bau2018.csv
  • MUd from Queensland (MUQ) by Kamber et al. (2005), being a semi-popular proxy for recent average detritus -- MUQ_Kamber2005.csv

Related Issue

Addresses point 2 of issue #35.

Motivation and Context

Pyrolite was previously missing widely-cited shale reference compositions, most notably PAAS. This PR adds those to pyrolite's already extensive reference composition database.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@kaarelmand
Copy link
Author

While I did add all these files to the refcomp folder, I am not sure if I have to run any scripts to re-populate the refdb.json file.

Nor am I sure how to update the documentation -- it seems to me that the page on the reference compositions is automatically populated?

@coveralls
Copy link

coveralls commented Mar 3, 2020

Pull Request Test Coverage Report for Build 1053

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 35 unchanged lines in 5 files lost coverage.
  • Overall coverage increased (+0.08%) to 86.632%

Files with Coverage Reduction New Missed Lines %
pyrolite/geochem/transform.py 2 95.6%
pyrolite/util/math.py 3 90.04%
pyrolite/util/synthetic.py 5 94.79%
pyrolite/plot/spider.py 8 92.31%
pyrolite/util/plot/axes.py 17 72.41%
Totals Coverage Status
Change from base Build 1013: 0.08%
Covered Lines: 4303
Relevant Lines: 4967

💛 - Coveralls

@morganjwilliams morganjwilliams self-requested a review March 4, 2020 00:37
@morganjwilliams morganjwilliams linked an issue Mar 4, 2020 that may be closed by this pull request
@morganjwilliams
Copy link
Owner

Thanks @kaarelmand, looks great!

I'll have a quick look at the CSVs, but I suspect this should be able to be merged without any issues. If you wanted to check they all import nicely, you could run the pyrolite.geochem tests locally (there's one included for the database update) - although note that the Travis build for the PR passed, and I can confirm that tests pass on my machine too!

To round off the PR, could you add a commit after running the update_database() function locally to add you new files to the refdb.json file? This is where pyrolite will look for the reference compositions.

@morganjwilliams
Copy link
Owner

On a slight tangent, I have a feeling that soon it will be worth writing up a tutorial about the variety of reference compositions, and their general applicability. Let me know if you're happy to contribute some info for a shale section!

@morganjwilliams
Copy link
Owner

The documentation example does indeed have an auto-documented list of the reference compositions, so there's no need to update this.

Given we now have 31 compositions, this will be something to break out into a new page soon. I've added #38 to this end, and we can address that separately.

@morganjwilliams morganjwilliams marked this pull request as ready for review March 4, 2020 05:11
Copy link
Owner

@morganjwilliams morganjwilliams left a comment

Choose a reason for hiding this comment

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

Looks good! These should be able to be directly merged into develop, and will be released with v0.2.6.

@kaarelmand
Copy link
Author

kaarelmand commented Apr 18, 2020

I'm in the process of fixing the .csv files I uploaded earlier. I also added a To Do list in the description.

@kaarelmand
Copy link
Author

kaarelmand commented Jul 3, 2020

Some musings, so I don't forget:

I'm thinking I should probably hold off on adding the conglomerates and sandstones from Condie, 1993 -- I don't know/think that anyone would actually need those compositions. And if someone does, it's very easy to add them in. Seems a bit silly in hindsight.

For the European shale, Bau et al. only report REY values, but there's some older data floating around for other elements on the same sample set. Nope, was all just REY values...

Sometime in the future, it might be worth thinking of a function that would take a time-series of element concentration data over Earth history, and normalize it to the changing crust in the Archean--Proterozoic. Condie, 1993, actually has 7 different composition brackets through Earth history, even though I only included his eon-averages.

@kaarelmand
Copy link
Author

All right, that should be it for this PR! Not sure if I still have to run any tests or whatnot?

@morganjwilliams
Copy link
Owner

There's no specific tests for the data files, but I'll pull down the fork and have a look locally.

@morganjwilliams
Copy link
Owner

@kaarelmand if you could fix the two reference cells for the NASC and PAAS data files I mentioned above and remove the extra text file I think this will then be ready to merge. I'll update the refdb.json after this - have an issue on my side which is likely due to a new version of TinyDB, and I'll update develop to fix this a the same time.

@kaarelmand
Copy link
Author

Whoops, these issues should be fixed now!

@morganjwilliams morganjwilliams merged commit c6d498b into morganjwilliams:develop Jul 7, 2020
@morganjwilliams
Copy link
Owner

Thanks for this one @kaarelmand! I had to re-update the reference database to pass the tests but everything else looks good. You can see some of the new compositions pop up in the normalization example.

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.

Convenience features for REE geochemistry and plotting [FEATURE]
3 participants