biocore / biocore/empress

Fix documentation / handling of Animator.gradientSteps

Open
#324 0 comments 0 reactions 0 assignees View on GitHub
bug refactoring
Dominant language
JavaScript
Stars
56
Forks
32
PR merge metrics
No merged PRs in 30d

Description

not a major thing or anything, but this attribute is described as an Object:

https://github.com/biocore/empress/blob/e3ed544cbab8cff7e5ea924848b3a67bfa569ce8/empress/support_files/js/animator.js#L56-L64

However as far as I can tell it's actually an Array, even though it's handled like it's an Object (see [here](https://github.com/biocore/empress/blob/e3ed544cbab8cff7e5ea924848b3a67bfa569ce8/empress/support_files/js/biom-table.js#L341-L358) for the implementation of `BiomTable.getUniqueSampleValues()`):

https://github.com/biocore/empress/blob/e3ed544cbab8cff7e5ea924848b3a67bfa569ce8/empress/support_files/js/animator.js#L134-L143

So, the docs should be corrected, and I think `this.totalFrames` can be created more simply via `this.totalFrames = this.gradientSteps.length;` (the `Object.keys()` thing still works because JS Arrays are technically Objects, so we get the same result but it's less efficient/straightforward).

Contributor guide

Open the contributing guide

Research direction

Read empress/support_files/js/animator.js at the documented gradientSteps attribute and totalFrames logic, then compare its handling with BiomTable.getUniqueSampleValues() in empress/support_files/js/biom-table.js. Done means the gradientSteps documentation identifies the actual type and the handling reflects the requested straightforward length-based behavior without changing the result.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
data-visualization
Issue type
Documentation
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.