racket / plot

Other
40 stars 37 forks source link

Add length to ticks contract #113

Closed bennn closed 1 year ago

bennn commented 1 year ago

ticks has a pretty good contract. For one, it'll catch layout functions that return bad lists:

ticks: contract violation
  expected: list?
  given: (list* (pre-tick 1673402400 #t) (pre-tick 1673614800 #t) 1673488800 #<datetime 2023-01-13T02:00:00>)
  in: the range of
      the 1st argument of
      (->
       (-> any/c any/c (listof pre-tick?))
       (-> any/c any/c any/c (listof string?))
       any)

But if the layout function returns a list of length 4 and the format function returns a list of length 2, there's an internal error:

map: all lists must have same size
  first list length: 4
  other list length: 2
  procedure: #<procedure:tick>
  context...:
   /Users/ben/code/racket/fork/racket/collects/racket/private/map.rkt:183:4: loop
   /Users/ben/code/racket/fork/racket/collects/racket/private/map.rkt:180:2: check-args
   /Users/ben/code/racket/fork/racket/collects/racket/private/map.rkt:257:2: gen-map
   /Users/ben/code/racket/fork/extra-pkgs/plot/plot-lib/plot/private/common/plot-element.rkt:25:2: default-ticks-fun
   /Users/ben/code/racket/fork/extra-pkgs/plot/plot-lib/plot/private/no-gui/plot2d-utils.rkt:51:0: get-ticks
   /Users/ben/code/racket/fork/extra-pkgs/plot/plot-lib/plot/private/no-gui/plot2d.rkt:43:0: plot/dc
   /Users/ben/code/racket/fork/extra-pkgs/plot/plot-lib/plot/private/no-gui/plot2d.rkt:171:0: plot-file
   ...cket/cmdline.rkt:191:51

Improve the contract --- or add a check.

Improve the docs for ticks-layout/c too. I wanted to return no string for minor ticks, but I guess "" is the right choice.

bennn commented 1 year ago

but I guess "" is the right choice

Not if you don't want adjacent ticks collapsed! (make-string i) is better.

alex-hhh commented 1 year ago

I agree that the documentation for the ticks-format function is confusing, and I created pull request #114 to try to improve it. Here is some example code that will trigger the new error:

#lang racket

(require plot)
(plot-new-window? #t)

(define (pr113-ticks-layout low high)
  (define n 10)
  (define step (/ (- high low) n))
  (for/list ([x (in-range n)])
    (pre-tick (+ low (* x step)) #t)))

(define (pr113-ticks-format low high pre-ticks)
  (define labels
    (for/list ([t (in-list pre-ticks)])
      (format "pr113 ~a" (pre-tick-value t))))
  ;; Skipping a label causes the ticks to fail, illustrating the new check in
  ;; `ticks-generate`
  (cdr labels))

(define pr113-ticks (ticks pr113-ticks-layout pr113-ticks-format))

(parameterize ([plot-x-ticks pr113-ticks])
  (plot (function sin -5 5)))

Unfortunately, I am not sure how to add a better contract for the length of the elements, so for now, I added a check to the ticks-generate function and report an error. This has the problem that the error is reported in the ticks-generate function, not in the user supplied function.

Unfortunately, the ticks-format/c contract as defined below is only used in the scribble documentation, not in the actual code, so adding a #post clause to the contract will have no effect, since this contract is not actually used:

https://github.com/racket/plot/blob/ccdcfa9f7b96407d1a7b40a61e622f6175b87f52/plot-lib/plot/private/common/leftover-contracts.rkt#L14

The "contract" for the format function is actually the Ticks-Format Typed Racket type defined below. Unfortunately, I don't know how to add a constraint to this type to specify that the length of the returned list has to be the same as the length of the input pre-tick list. @samth do you have any ideas?

https://github.com/racket/plot/blob/ccdcfa9f7b96407d1a7b40a61e622f6175b87f52/plot-lib/plot/private/common/ticks.rkt#L32****

bennn commented 1 year ago

A check inside the function sounds good.

TR won't be able to check list length without enabling refinements.