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

function decompact_linear #37

Open
NPJuncal opened this issue Jan 15, 2024 · 7 comments
Open

function decompact_linear #37

NPJuncal opened this issue Jan 15, 2024 · 7 comments

Comments

@NPJuncal
Copy link
Collaborator

Function to estimate sample depth (min and max) after matematical decompresion.

Input is a df with core code_id, compresion % and min and max depth of each sample. Output is same df with tow new columns with corrected sample min and max depth. Same correction is applied to all samples (linear correction).

tasks:
-check if it works
-write documentation
-write tests

enhancement 1: if compresion is not estimate a additional df with the field mesurements can be provided to estimate core compresion for each core using function estimate_compresion.

enhancement 2: allow to give a column with dry bulk density (compresed) and estimate decompresed dry bulk density at same time

@NPJuncal
Copy link
Collaborator Author

NPJuncal commented Jan 18, 2024

It works with example data
enhancement 1 has been implemented

tasks:
-there are two input dataframes. Right now it only checks the columns and allow the user to provide the name of the columns of one of them. It needs to allow to modify the field measurements dataframe and them transform the names so it can work as an input to the function estimate_compaction
-the df_fm is optional, the function cannot break if it is not provided
-write documentation
-write tests

enhancement 2: allow to give a column with dry bulk density (compresed) and estimate decompresed dry bulk density at same time

@MarcioFCMartins
Copy link
Collaborator

Enhancement 2 was addressed. I made several changes that I think make the function more readable. I'll write some tests when I have the time, it'll be good for me to learn how to do those :)

@NPJuncal
Copy link
Collaborator Author

Hy @Pakillo @MarcioFCMartins @costavale @Julenasti, I have a question about how to approach this function.

I am working in the first improvement. Right now you have two input dataframes:

  • The dataframe with the depth and dry bulk density data to be corrected for compresion
  • An optional database with field measurements to estimate the compresion

The idea is that if you have cores without the compression estimated, the function will get data from the second dataframe and estimate the compression of those cores using the estimate_compaction function (one of the other main functions of the package, that already works). However, this complicates the function quite a bit, and I am having trouble allowing the user to choose the names of the input columns in the second dataframe. If the dataframes are provided with the default column names for the second dataframe, it works (you already can choose the names of the first dataframe columns with no problem).

I have pushed what I have done until now to github so you can check it.

My question is: if there is already a function to estimate compresion in the package, does it make sense to include this process within the other function? Wouldn't it be better to just run the estimate_compression function for all your cores and then the decompact_linear function?

That way the function would be way more simple and robust, right?
What do you think? I am sure it would not take much of your (or mine) time to make it work with both dataframes, but maybe it does not make sense.

@MarcioFCMartins
Copy link
Collaborator

MarcioFCMartins commented Mar 5, 2024 via email

@costavale
Copy link
Collaborator

I agree, let's keep estimate_compression separated. We will write a workflow for the users in the vignette of the package

@NPJuncal
Copy link
Collaborator Author

NPJuncal commented Mar 7, 2024

Perfect! I have changed the function to eliminated the field measurements dataframe and the code to estimate compression within the function. Now it works :)

tasks:

-write documentation
-write tests

@NPJuncal
Copy link
Collaborator Author

Both test and documentation are written

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

No branches or pull requests

3 participants