biocore / biocore/empress

Precompute constant in arc approximation function

Open
#438 0 comments 0 reactions 0 assignees View on GitHub
good first issue performance
Dominant language
JavaScript
Stars
56
Forks
32
PR merge metrics
No merged PRs in 30d

Description

Discussed with @kwcantrell earlier today. In the following function:

https://github.com/biocore/empress/blob/26f1b02caa004c7db5e4e281681804b34f124622/empress/support_files/js/empress.js#L595-L598

It should be possible to rewrite the first line as

```js
var numSamples = Math.floor(Math.abs(19.09859 * totalAngle));
```

... since the `60 / pi` value will always be the same. This may save us some time, since in theory this function is called once for every internal node when doing the circular layout; Kalen brought up that if JS' interpreter is smart enough it may already be doing this behind the scenes, but at least this way we will make the code a bit clearer.

When we make this change, we should also add a comment explaining things / linking to this issue so the person reading the code knows where the 19.09859... figure came from.

Contributor guide

Open the contributing guide

Research direction

Open empress/support_files/js/empress.js around lines 595-598 and inspect the arc approximation function. Replace the repeated constant calculation with the proposed precomputed value and add a comment explaining its origin or linking to issue #438. Done means the circular-layout calculation uses the constant while preserving the existing behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
data-visualization
Issue type
Refactor
Difficulty
1/5
Estimated time
Under an hour
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.