Skip to content

add xyObjectPivot function and corresponding #294

Closed
jobo322 wants to merge 2 commits intomainfrom
xy-object-pivot-10-51
Closed

add xyObjectPivot function and corresponding #294
jobo322 wants to merge 2 commits intomainfrom
xy-object-pivot-10-51

Conversation

@jobo322
Copy link
Member

@jobo322 jobo322 commented Feb 20, 2025

No description provided.

@codecov
Copy link

codecov bot commented Feb 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.40%. Comparing base (bd9dc3e) to head (63fcc4c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #294      +/-   ##
==========================================
+ Coverage   96.38%   96.40%   +0.01%     
==========================================
  Files         184      185       +1     
  Lines        5152     5176      +24     
  Branches     1147     1155       +8     
==========================================
+ Hits         4966     4990      +24     
  Misses        186      186              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines +34 to +35
const { thresholdFactor = 0.2, fromTo = { from: minX, to: maxX } } = options;
let { from, to } = fromTo;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach seems wrong because if you give fromTo={from:1} to will never be defined to the correct value.

But in this project ml-spectra-processing we don't use an object fromTo but 2 properties from and to. Also from / to are the values. If you want to index you need to use fromIndex and toIndex.

You can also allow the 4 parameters and use the helper function:

const { fromIndex, toIndex } = xGetFromToIndex(array, options);
let minValue = array[fromIndex];

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but the interface will alert if you pass only from in the object, should I add a guard for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, if I assume than x is ordered we could ignore the maxX minX generation, but I need the x value instead of index because the x value is in ppm scale

const { thresholdFactor = 0.2, fromTo = { from: minX, to: maxX } } = options;
let { from, to } = fromTo;

if (from > to) [from, to] = [to, from];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The switch of value is manage by xGetFromToIndex

}

/**
* Finds the index of the peak in an array of points (x, y) based on a threshold factor in Y values and a specified range in X values.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what this method does. What is special with this peak is not written in the description and you return only one.

… instead of index and adjust tests accordingly
@lpatiny lpatiny closed this Feb 20, 2025
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

Successfully merging this pull request may close these issues.

2 participants