Closed ValentineHerr closed 1 year ago
@VincyaneBadouard, I saw you changed DHB
to Diameter
with this commit.
I am still not convinced about it (and have not made the change on my end). But even if we do change DBH to Diameter, the workflow should be:
input: Some_Column
--> RequiredFormat --> Diameter
[in cm] + DiameterOriginal
--> (optional: Correction functions --> Diameter
[in cm and, e.g. corrected to be at constant hight, and whatever other corrections are selected] + DiameterOriginal
) --> ReversedRequiredFormat --> Some_other_Column_name
[based on profile selected as output, in units also based on that output profile] + DiameterOriginal
In other words, I do not want new columns like DBH or DBHCorr, because I need to read Diameter
to be able to map to the output profile. Since we will save the original diameter in DiameterOriginal
, we do not need intermediary columns. The user will know what correction he/she selected and what the new Diameter means.
Also, for HOM
, the workflow should be:
input: Some_Column
--> RequiredFormat --> HOM
[in m] + HOMOriginal
--> _(optional: Correction functions --> HOM
[all equal 1.3 - or custum value - IF taper correction is selected, in cm] + DiameterOriginal
)_ --> ReversedRequiredFormat --> Some_other_Column_name
[based on profile selected as output, in units also based on that output profile] + DiameterHOM
Does that make sense?
Conclusion, at least for now, is to only do unit transformations in columns that are given as input. Anything that is happening or changed in the corrections function should happen in copies of input columns named as: "DiameteCorrectedByApp", "HOMCorrectedByApp", "LifeStatusCorrectedByApp"...
@VincyaneBadouard please, apply these names in your function and make only those columns get overwritten. You can close this issue once you've read this message and agree with it.
Also, @VincyaneBadouard, I'll need to know all those columns so the function RequiredFormat gives them to you. So if you have functions that deal with other columns than Diameter, HOm and LiveStatus, please let me know.
We will also have to work carefully on the metadata so it is as explicit as possible + work in the the comments
columns you add to the data.
Conclusion, at least for now, is to only do unit transformations in columns that are given as input. Anything that is happening or changed in the corrections function should happen in copies of input columns named as: "DiameteCorrectedByApp", "HOMCorrectedByApp", "LifeStatusCorrectedByApp"...
@VincyaneBadouard please, apply these names in your function and make only those columns get overwritten. You can close this issue once you've read this message and agree with it.
@VincyaneBadouard, when you get a chance, can you change your column names created by your corrections functions to be more explicit as, like @gabrielareto had pointed out DBHCorr
is likely to be already used by the user. Appending CorrectedByApp
, even though long, sounds self-explanatory and safe to me.
When done, you can close this issue.
After some discussion with @VincyaneBadouard we thought it made more sense to ask for Diameter rather that DBH, since HOM is not always 1.3 m :
DBH
butDiameter
(and of course also ask for HOM and POM).Diameter
as output, so @VincyaneBadouard would useDiameter
as input for her correction function, to create columnDBH
.DBH
as input for that one? If that is the case, the output should only have columnHOMOriginal
and noHOM
, right? since at that point all DBH will be given at 1.3m for all trees.... but what if taper correction is not selected? that causes issues to me...We need to discuss more about this. To me, the original plan was probably best: we keep asking for DBH (it is known that it is not always measured at 1.3m, that is why HOM is always measured). so in the output,
DBHOriginal
will be DBH as given in input andDBH
will be the corrected DBH, if any correction involving DBH was selected.@VincyaneBadouard, please let me know if the original plan makes sense to you.