feat: proposal for IV spectrum #63
Conversation
| /** | ||
| * | ||
| */ | ||
| /** Scale representation of variable */ |
There was a problem hiding this comment.
what would be the usecase of this? i'm afraid that this might be confusing as you do not know if this means that it is scaled, should be scaled, or was sampled on this scale
There was a problem hiding this comment.
This is one of the metadata that we save for each variable that is specified by the user for how this variable should be visualized (for example in react-plot)
There was a problem hiding this comment.
that's a good point! should one generalize this (some things might be better on ln, others on log10, others on sqrt, ...) but not sure how to represent this diversity
There was a problem hiding this comment.
that's another good point, those could be the possible options for the moment
type scale = 'linear' | 'ln' | 'log10' | 'sqrt';
There was a problem hiding this comment.
I don't think that it is related to the data. This is something to decide at the visualization level.
There was a problem hiding this comment.
But those decisions are generally taken as a standard, and this I think is a piece of valuable information for a processing step, adding how it's planned to be analyzed.
There was a problem hiding this comment.
I think this is key distinction about this repo. The types here are supposed to focus on data, i.e., things that have been done/measured/...
I agree that there is value in recording such information, e.g., processing preference. However, This is something that might be specific to the frontend library (?)
|
|
||
| interface RegressionScore { | ||
| r: number; | ||
| r2: number; |
There was a problem hiding this comment.
r2 is not uniquely defined, there are multiple ways to calculate this. (Which is true in general for metrics). I think one might want to also store something like "method" (as we try to do in the derivedProperty)
There was a problem hiding this comment.
Even if the method is not defined, the analysis of the resulting information should be standard in the same database. Do you think that will still give more information to know the method?
There was a problem hiding this comment.
I think if you always assume that it is calculated by your function than this is true. But if you also want to reflect that this might be deposited by some service, this might not be that clear
There was a problem hiding this comment.
@lpatiny what do you think?
i feel it be an implementation of a more general derivedproperty
cheminfo-types/todo/spectra/core/BaseDerivedProperty.d.ts
Lines 9 to 13 in 52c3e13
| surfaceArea?: Value | ||
| } | ||
| derived?: { | ||
| thresholdVoltage?: ValueXY; |
There was a problem hiding this comment.
what would be the x and y for the voltage?
There was a problem hiding this comment.
@lpatiny maybe we should continue adding examples. I think you didn't like having them in the docstring with the @ but i feel it would be really useful to understand what is intended
There was a problem hiding this comment.
In this case, I'm describing a point with the ValueXY. The threshold voltage is the voltage where the Id > 1uA/mm (this current could change, that's why is better to return also this value, even if the voltage is the important one)
There was a problem hiding this comment.
@lpatiny maybe we should continue adding examples. I think you didn't like having them in the docstring with the
@but i feel it would be really useful to understand what is intended
Yes indeed we should add more @example https://tsdoc.org/pages/tags/example/
maasencioh
force-pushed
the
iv-spectrum
branch
from
32de4fc
to
0bac433
Compare
No description provided.