Invalid `floor` invocation from `isTimeInterval`

Open Beginner friendly
#2,447 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
1/5
Estimated time
Under an hour
Newbie friendliness
85/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
javascript, typescript
Domain
data-visualization

Research direction

Start with the floor call in src/options.js and compare it with the floor signature in src/interval.d.ts. Update the type contract to reflect the no-argument invocation, then verify that the declaration matches the documented binning behavior and type checking succeeds.

Written by the indexing model from the issue text.

Description

Somewhere during binning of temporal data this function is executed:

https://github.com/observablehq/plot/blob/356f579b1d947ee05a914420eddff0f29cee300a/src/options.js#L428-L430

Notice that no argument is passed to floor when the function tries to figure out if the result type is a Date.

This is in conflict with the type definitions, which indicate that a parameter to floor is guaranteed:

https://github.com/observablehq/plot/blob/356f579b1d947ee05a914420eddff0f29cee300a/src/interval.d.ts#L34-L47

I believe the simplest fix would be

-   */
-  floor(value: T): T
+   *
+   * If no value is passed, the function is expected to return *any* result
+   * matching its return type.
+   */
+  floor(value?: T): T

Related: #2423

Edit: I just noted that this behaviour is documented on the Bin transform page (but not the Interval transform), which increases my confidence that it’s really just the type that’s off.

Time intervals are intervals that are also functions that return a Date instance when called with no arguments.

Dominant language
HTML
Stars
5.4k
Forks
244
No merged PRs in 30d

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from observablehq/plot

All issues in observablehq/plot

Similar issues

More Data Visualization issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.