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

Pu/vectorization #673

Merged
merged 21 commits into from
May 28, 2021
Merged

Pu/vectorization #673

merged 21 commits into from
May 28, 2021

Conversation

izulin
Copy link
Collaborator

@izulin izulin commented May 18, 2021

Context

Vectorization of scalar functions.

How has this been tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature or improvement (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Additional language file or change to the existing one (translations)

Related issue(s):

Checklist:

  • My code follows the code style of this project,
  • My change requires a change to the documentation,
  • I described the modification in the CHANGELOG.md file.

@izulin izulin changed the base branch from master to pu/matrixinput May 18, 2021 12:54
@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 18, 2021

This pull request introduces 39 alerts and fixes 12 when merging 98ad66d into 7ff3ab7 - view on LGTM.com

new alerts:

  • 38 for Unused variable, import, function or class
  • 1 for Unreachable statement

fixed alerts:

  • 11 for Unused variable, import, function or class
  • 1 for Useless assignment to property

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 18, 2021

This pull request introduces 38 alerts when merging 98ad66d into 027597f - view on LGTM.com

new alerts:

  • 38 for Unused variable, import, function or class

@codecov
Copy link

codecov bot commented May 18, 2021

Codecov Report

Merging #673 (8d0c3c5) into develop (94c3d72) will decrease coverage by 0.09%.
The diff coverage is 91.11%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #673      +/-   ##
===========================================
- Coverage    92.36%   92.27%   -0.10%     
===========================================
  Files          166      166              
  Lines        39089    39163      +74     
  Branches      5349     5355       +6     
===========================================
+ Hits         36106    36138      +32     
- Misses        2947     2986      +39     
- Partials        36       39       +3     
Impacted Files Coverage Δ
src/MatrixSize.ts 48.48% <34.48%> (-2.58%) ⬇️
src/interpreter/plugin/FunctionPlugin.ts 80.12% <75.70%> (-2.93%) ⬇️
src/interpreter/plugin/BooleanPlugin.ts 97.36% <88.46%> (-0.96%) ⬇️
src/interpreter/plugin/AbsPlugin.ts 100.00% <100.00%> (ø)
src/interpreter/plugin/ArrayPlugin.ts 100.00% <100.00%> (ø)
src/interpreter/plugin/BitShiftPlugin.ts 98.48% <100.00%> (ø)
...interpreter/plugin/BitwiseLogicOperationsPlugin.ts 100.00% <100.00%> (ø)
src/interpreter/plugin/CharPlugin.ts 95.83% <100.00%> (ø)
src/interpreter/plugin/CodePlugin.ts 100.00% <100.00%> (ø)
src/interpreter/plugin/ComplexPlugin.ts 100.00% <100.00%> (ø)
... and 37 more

@izulin izulin requested a review from voodoo11 May 18, 2021 14:48
@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 19, 2021

This pull request introduces 38 alerts when merging 29177f4 into b0dc57f - view on LGTM.com

new alerts:

  • 38 for Unused variable, import, function or class

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 24, 2021

This pull request introduces 38 alerts when merging 23ae7d7 into be763b4 - view on LGTM.com

new alerts:

  • 38 for Unused variable, import, function or class

CHANGELOG.md Outdated Show resolved Hide resolved
@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 25, 2021

This pull request introduces 38 alerts when merging 8a00d3f into be763b4 - view on LGTM.com

new alerts:

  • 38 for Unused variable, import, function or class

Comment on lines 58 to 61
/**
* Some function do not allow vectorization: array-output, and special functions.
*/
vectorizationForbidden?: boolean,
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be added to the docs: https://handsontable.github.io/hyperformula/guide/custom-functions.html#implementedfunction-property

I think we forgot to update, more properties are missing :|

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are subtle differences between parameters that are used by the core engine, and parameters that are just a guiding info for our wrappers.
That is, unless the custom function is written in a manner that supports vectorization, NOT setting vectorizationForbidden will not produce valid result anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

And yet, all those parameters may be used for custom function, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, they are all passed. But some of those have no effect unless explicitly used by the function implementation.

Base automatically changed from pu/matrixinput to develop May 27, 2021 18:15
@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 27, 2021

This pull request introduces 38 alerts when merging 6b5092d into 94c3d72 - view on LGTM.com

new alerts:

  • 38 for Unused variable, import, function or class

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 27, 2021

This pull request introduces 38 alerts when merging 8d0c3c5 into 94c3d72 - view on LGTM.com

new alerts:

  • 38 for Unused variable, import, function or class

Copy link
Contributor

@wojciechczerniak wojciechczerniak left a comment

Choose a reason for hiding this comment

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

👍

@izulin izulin merged commit b01927e into develop May 28, 2021
@izulin izulin deleted the pu/vectorization branch May 28, 2021 12:46
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.

None yet

2 participants