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

Fix TAS field names in json files #92

Merged

Conversation

bobmyhill
Copy link
Contributor

@bobmyhill bobmyhill commented Sep 20, 2023

Description

The json files containing the TAS information had a couple of errors:

  • The alkalic (Ba) and subalkalic (Bs) basalt fields were given the same names (Basalt and Gabbro). These have been changed to Alkalic/Subalkalic Basalt/Gabbro.
  • The trachybasalt-trachyandesite series (S1, S2, S3) intrusive rocks were all given the name "foidolite". These have been changed, following Middlemost (1994), to Monzo-gabbro, Monzo-diorite and Monzonite.
  • The "Peridotitgabbro" field has been renamed to "Peridotgabbro", following Middlemost (1994).

I also took the opportunity to linebreak a few more labels so that they fit better into the corresponding fields.

To make code-review easier, I first formatted the three files (first commit) and then made the changes (second commit). See here for the various name changes: 4f3b189

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project.
  • All new and existing tests passed.

@morganjwilliams morganjwilliams self-requested a review September 21, 2023 01:22
@morganjwilliams morganjwilliams added the bug Something isn't working label Sep 21, 2023
@bobmyhill bobmyhill marked this pull request as draft September 21, 2023 08:08
@bobmyhill bobmyhill changed the base branch from main to develop September 21, 2023 08:12
@bobmyhill bobmyhill marked this pull request as ready for review September 21, 2023 08:12
@bobmyhill bobmyhill force-pushed the fix_intrusive_TAS_labels branch from 214fe04 to 4f3b189 Compare September 21, 2023 09:25
@coveralls
Copy link

Pull Request Test Coverage Report for Build 6259780433

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.006%) to 90.411%

Totals Coverage Status
Change from base Build 6010078958: -0.006%
Covered Lines: 5978
Relevant Lines: 6612

💛 - Coveralls

@bobmyhill
Copy link
Contributor Author

This PR passes with 3.8, the failures with 3.9+ don't seem to be related to this PR. Looks like the failures are due to changes in other modules?

@bobmyhill
Copy link
Contributor Author

Fixes to the failing tests provided in #96.

@morganjwilliams
Copy link
Owner

These look good, I might look to reformat the JSON a little down the line but your two commits do indeed make it easier to see the change!

@morganjwilliams morganjwilliams merged commit dad53c6 into morganjwilliams:develop Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants