Repository navigation
Adds Magnetite trace element classification and discriminant diagram - #129
Conversation
|
Thanks for putting this through, @jarrod-burges! Away from the desk at the moment but will have a read through in the next day or two. Also appreciate that you've followed the style of what was there already, should make for an easy merge. |
|
Thanks, @morganjwilliams. I'd also like to do some other diagrams, like the IOCG one that was mostly completed but never merged. Let me know if there are other issues or things to fix. |
morganjwilliams
left a comment
morganjwilliams
left a comment
There was a problem hiding this comment.
Hey @jarrod-burges, a few comments here and there - main open question is around the log-log plot accessor, I think we can accommodate it in a more general way via init_axes(). Have a quick read as suits and let me know your thoughts.
| data = data.loc[mask] | ||
|
|
||
| size = data.index.size | ||
| kw = process_color(size=size, **kwargs) |
There was a problem hiding this comment.
Using on the return of this in the scatter call will strip e.g. marker sizes and anything which isn't processed by process_color explicitly
There was a problem hiding this comment.
Planning to address a few more issues in process_color to make it work more nicely with modern matplotlib yet (some of the reasons it existed they've since addressed).
There was a problem hiding this comment.
Since the code now uses the base plot and passes either linear or log scaling this shouldn't be an issue now right?
…axes. This is then updated in the magnetite template plot.
Coverage Report for CI Build 43090573181Coverage decreased (-0.2%) to 90.543%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
|
@morganjwilliams The comments have been addressed. The plots still result in the previous images. The log-log plot has been removed and adjusted to use init_axes() and passes either 'linear' (default) or 'log'. So the other plots still work as expected too. |
| import matplotlib.text | ||
| import numpy as np | ||
| import pandas as pd | ||
| from jinja2.nodes import Literal |
There was a problem hiding this comment.
Think this was an autoimport which went awry (had lots of these when I introduced more typing) - I'll fix this up after merging.
There was a problem hiding this comment.
Yeah I was going to do Literals for the linear or log selection to force either or without declaring enumeration. however it would’ve limited the python version and matplotlib just checks static strings, so I figured that would be enough.
|
Thanks @jarrod-burges, I'll merge over to develop now; let me know if you're happy for me to add you to the contributors and link to your GH account. |
That sounds good. Thanks! |
Description
Adds the feature requested in #106, creates "Ca + Al + Mn vs Ti + V" and "Ni/(Cr + Mn) vs Ti + V" plots. Pyrolite did not have a log-log plot, so that was added.
Related Issue
Related issue #106
Motivation and Context
This solves analyzing and plotting magnetite trace element data for ore geology studies.
Types of changes
Checklist: