Oxideconversion

[Abagna123]

No description provided.

[archaeothommy]

@Abagna123, the following limitations of your functions are a no go:

• "Conversion factors must be provided.": It is not job of the user to provide the conversion factors and atomic weigts. They must come with ASTR. If you are not sure how to include data in a package, read https://r-pkgs.org/data.html
• "Overwrites output if different elements map to the same oxide": They must be handled, e.g. by summarising them into one column. The existing draft for conversion to oxide% includes a suggestion how to do this. The same applies for the other direction: If e.g. Fe2O3 and FeO are given, the output must include one column "Fe".

In addition, please add:

• Include normalisation to 100% of the converted values as optional parameter (default value: FALSE)
• Specific for the conversion to oxide%:
  • Include the funtionality to choose which oxide should be used if multiple oxides exist for an element. It was originally suggested that this could be a rather rough approach, having pre-compiled lists with the typical oxides for the respective environment (e.g., FeO for reduced atmosphere, Fe2O3 for oxidising atmosphere; see the "default oxides" list here). However, something more fine-grained would be desirable, e.g. an input parameter taking a vector with the list of oxides, labels as short-cut for pre-compiled lists (e.g. "reducing" or "oxidising") and maybe even a value "interactive" to enter interactive mode if the user wants to choose for each element which oxide to use (see readline()).
  • Conversion to oxide%: include the option to convert all, only major, or only minor elements, as already included in the existing draft

(See the already existing work here, which has already some of the functionalities currently missing in your functions)

"No bulk conversion; each conversion must be explicitly requested.": I am not sure what you are referring to here with "bulk", please explain.

[Abagna123]

The conversion factors are built into the package (users don't need to provide them). If there are duplicate oxides/elements (like FeO and Fe₂O₃), they get summed automatically into one column. There's an option to normalize everything to 100% (turned off by default). You can choose which oxide to use "reducing" (gives FeO), "oxidizing" (gives Fe₂O₃), "interactive" (asks you), or pick your own. Also, can filter to show only major elements (>1%) or minor elements (>0.1%). you can convert multiple elements/oxides at once and new columns get added.

About the "bulk conversion" comment. In my first draft, you had to convert each element one by one (which was wrong). Now the functions let you convert everything at once.

[archaeothommy]

Thank you @Abagna123, this looks already very good.
I only managed to go through the wt_to_at() function. I slightly revised it and added the option that the original columns are dropped (sometimes, you want just the converted values). I also unified the data objects. There is no need to split the conversion table into three if all functions can easily draw data from the same table instead.

I'm worried that the calculations are not correctly done. When using the wt%-composition of quart from mineralienatlas.de, I get as result 60.64852 at% Si and 39.35148 at% O. This is quite far off from their values of 66.6 at% Si and 33.3 at% O... The values in the table are correct. Please check this!

Please revise at_to_wt() according to my changes in wt_to_at(). And please adjust the behavior when checking the input. wt_to_at() currently returns the original data frame without any further information when elements in it do not match the chemical elements in the conversion table. at_to_wt() throws an error if there are missing columns. The desired behaviour is an error if column names contain values that are not listed in the reference table.

Finally, please refrain from using [skip-ci]. The tests are there for a reason and must not be skipped when you push something that is considered ready. If your commit is not passing them, it means that it is not ready yet. See the documents in the dev_guide folder on how to run the checks on your computer to make sure they are all passed before pushing something.

[archaeothommy]

@Abagna123, to ensure internal consistency, please add to the conversion table:

• elements (symbol, if available atomic weight, all other values empty): "Ac" "Am" "Ar" "At" "Au" "Bh" "Bk" "Br" "Cf" "Cl" "Cm" "Ds" "Db" "Dy" "Es" "F" "Fm" "Fr" "He" "Hs" "I" "Ir" "Kr", "Lr" "Md" "Mt" "Nb" "Ne" "No" "Np" "Pa" "Po" "Pt" "Ra" "Rf" "Rg" "Rn" "Sg" "Tc"
• oxides: "Cl2O" "Cl2O7" "ClO2" "OF2", "Au2O3"

[archaeothommy]

@Abagna123 :

Conversion table: Where are the conversion factors for the newly added oxides?

Conversion element -> oxide: I specified in the documentation the expected behaviour when I had the impression it was not clear enough and made suggestions for more efficient ways to carry out the tasks in it and its underlying functions. Please see the comments in the functions for more information on what the missing steps should be.