search

nextjournal / clerk-table-stats

Add simple columns summaries to your Clerk tables
ISC License
0 stars 0 forks source link

Stats viewer throws a NullPointerException on certain tabular data #12

Closed zampino closed 4 months ago

zampino commented 4 months ago

To reproduce, uncomment the viewer in this notebook, simply starting ductile with bb dev.

Execution error (NullPointerException) at nextjournal.clerk-table-stats/transform-fn (clerk_table_stats.clj:542).
Cannot invoke "clojure.lang.Named.getNamespace()" because "x" is null
java.lang.NullPointerException: Cannot invoke "clojure.lang.Named.getNamespace()" because "x" is null
             core.clj:1618 clojure.core/namespace
             core.clj:1612 clojure.core/namespace
 clerk_table_stats.clj:542 nextjournal.clerk-table-stats/transform-fn
 clerk_table_stats.clj:541 nextjournal.clerk-table-stats/transform-fn
          viewer.cljc:1466 nextjournal.clerk.viewer$apply_viewers_STAR_.invokeStatic
          viewer.cljc:1459 nextjournal.clerk.viewer$apply_viewers_STAR_.invoke
          viewer.cljc:1661 nextjournal.clerk.viewer$present_STAR_.invokeStatic
          viewer.cljc:1654 nextjournal.clerk.viewer$present_STAR_.invoke
          viewer.cljc:1614 nextjournal.clerk.viewer$present_PLUS_paginate_children$fn__54793.invoke
             core.clj:7490 clojure.core/map-indexed[fn]
         protocols.clj:167 clojure.core.protocols/fn
         protocols.clj:123 clojure.core.protocols/fn
          protocols.clj:19 clojure.core.protocols/fn[fn]
          protocols.clj:31 clojure.core.protocols/seq-reduce
          protocols.clj:74 clojure.core.protocols/fn
          protocols.clj:74 clojure.core.protocols/fn
          protocols.clj:13 clojure.core.protocols/fn[fn]
             core.clj:7026 clojure.core/transduce
             core.clj:7042 clojure.core/into
             core.clj:7029 clojure.core/into
          viewer.cljc:1612 nextjournal.clerk.viewer$present_PLUS_paginate_children.invokeStatic
          viewer.cljc:1603 nextjournal.clerk.viewer$present_PLUS_paginate_children.invoke
          viewer.cljc:1676 nextjournal.clerk.viewer$present_STAR_.invokeStatic
          viewer.cljc:1654 nextjournal.clerk.viewer$present_STAR_.invoke
          viewer.cljc:1614 nextjournal.clerk.viewer$present_PLUS_paginate_children$fn__54793.invoke
             core.clj:7490 clojure.core/map-indexed[fn]
             core.clj:2894 clojure.core/take[fn]
             core.clj:2942 clojure.core/drop[fn]
         protocols.clj:167 clojure.core.protocols/fn
         protocols.clj:123 clojure.core.protocols/fn
          protocols.clj:19 clojure.core.protocols/fn[fn]
          protocols.clj:31 clojure.core.protocols/seq-reduce
          protocols.clj:74 clojure.core.protocols/fn
          protocols.clj:74 clojure.core.protocols/fn
          protocols.clj:13 clojure.core.protocols/fn[fn]
             core.clj:7026 clojure.core/transduce
             core.clj:7042 clojure.core/into
             core.clj:7029 clojure.core/into
          viewer.cljc:1612 nextjournal.clerk.viewer$present_PLUS_paginate_children.invokeStatic
          viewer.cljc:1603 nextjournal.clerk.viewer$present_PLUS_paginate_children.invoke
          viewer.cljc:1676 nextjournal.clerk.viewer$present_STAR_.invokeStatic
          viewer.cljc:1654 nextjournal.clerk.viewer$present_STAR_.invoke
          viewer.cljc:1614 nextjournal.clerk.viewer$present_PLUS_paginate_children$fn__54793.invoke
             core.clj:7490 clojure.core/map-indexed[fn]
             core.clj:2894 clojure.core/take[fn]
             core.clj:2942 clojure.core/drop[fn]
         protocols.clj:167 clojure.core.protocols/fn
         protocols.clj:123 clojure.core.protocols/fn
          protocols.clj:19 clojure.core.protocols/fn[fn]
          protocols.clj:31 clojure.core.protocols/seq-reduce
          protocols.clj:74 clojure.core.protocols/fn
          protocols.clj:74 clojure.core.protocols/fn
          protocols.clj:13 clojure.core.protocols/fn[fn]
             core.clj:7026 clojure.core/transduce
             core.clj:7042 clojure.core/into
             core.clj:7029 clojure.core/into
          viewer.cljc:1612 nextjournal.clerk.viewer$present_PLUS_paginate_children.invokeStatic
          viewer.cljc:1603 nextjournal.clerk.viewer$present_PLUS_paginate_children.invoke
          viewer.cljc:1676 nextjournal.clerk.viewer$present_STAR_.invokeStatic
          viewer.cljc:1654 nextjournal.clerk.viewer$present_STAR_.invoke
          viewer.cljc:1759 nextjournal.clerk.viewer$present.invokeStatic
          viewer.cljc:1751 nextjournal.clerk.viewer$present.invoke
           viewer.cljc:533 nextjournal.clerk.viewer$transform_result.invokeStatic
           viewer.cljc:517 nextjournal.clerk.viewer$transform_result.invoke
          viewer.cljc:1466 nextjournal.clerk.viewer$apply_viewers_STAR_.invokeStatic
          viewer.cljc:1459 nextjournal.clerk.viewer$apply_viewers_STAR_.invoke
          viewer.cljc:1661 nextjournal.clerk.viewer$present_STAR_.invokeStatic
          viewer.cljc:1654 nextjournal.clerk.viewer$present_STAR_.invoke
          viewer.cljc:1614 nextjournal.clerk.viewer$present_PLUS_paginate_children$fn__54793.invoke
             core.clj:7490 clojure.core/map-indexed[fn]
 PersistentVector.java:418 clojure.lang.PersistentVector.reduce
             core.clj:7025 clojure.core/transduce
             core.clj:7042 clojure.core/into
             core.clj:7029 clojure.core/into
          viewer.cljc:1612 nextjournal.clerk.viewer$present_PLUS_paginate_children.invokeStatic
          viewer.cljc:1603 nextjournal.clerk.viewer$present_PLUS_paginate_children.invoke
          viewer.cljc:1676 nextjournal.clerk.viewer$present_STAR_.invokeStatic
          viewer.cljc:1654 nextjournal.clerk.viewer$present_STAR_.invoke
          viewer.cljc:1759 nextjournal.clerk.viewer$present.invokeStatic
          viewer.cljc:1751 nextjournal.clerk.viewer$present.invoke
             core.clj:2586 clojure.core/comp[fn]
             core.clj:2759 clojure.core/map[fn]
             core.clj:7806 clojure.core/preserving-reduced[fn]
 PersistentVector.java:418 clojure.lang.PersistentVector.reduce
             core.clj:6964 clojure.core/reduce
             core.clj:7817 clojure.core/cat[fn]
             core.clj:2759 clojure.core/map[fn]
 PersistentVector.java:418 clojure.lang.PersistentVector.reduce
             core.clj:7025 clojure.core/transduce
             core.clj:7042 clojure.core/into
             core.clj:7029 clojure.core/into
             core.clj:2648 clojure.core/partial[fn]
             core.clj:6259 clojure.core/update
             core.clj:6251 clojure.core/update
          viewer.cljc:1278 nextjournal.clerk.viewer$process_blocks.invokeStatic
          viewer.cljc:1274 nextjournal.clerk.viewer$process_blocks.invoke
             core.clj:2641 clojure.core/partial[fn]
             core.clj:6259 clojure.core/update
             core.clj:6251 clojure.core/update
          viewer.cljc:1307 nextjournal.clerk.viewer$fn__54698.invokeStatic
          viewer.cljc:1305 nextjournal.clerk.viewer$fn__54698.invoke
             clerk.clj:105 ductile.clerk/fn
              clerk.clj:99 ductile.clerk/fn
          viewer.cljc:1466 nextjournal.clerk.viewer$apply_viewers_STAR_.invokeStatic
          viewer.cljc:1459 nextjournal.clerk.viewer$apply_viewers_STAR_.invoke
          viewer.cljc:1661 nextjournal.clerk.viewer$present_STAR_.invokeStatic
borkdude commented 4 months ago

@zampino This happens in the expression:

var-name (symbol (namespace id) (str (name id) "-table"))

Apparently the wrapped-value that comes in doesn't have an id. What to do in that case?

borkdude commented 4 months ago

I'll try to repro tomorrow

zampino commented 4 months ago

yeah, the cell id appears to be nil (or missing) in some cases, would be cool to know for which cells. Probably some recursive call of apply-viewers on some nested structure which are not cells.

zampino commented 4 months ago

Probably some recursive call of apply-viewers on some nested structure which are not cells.

In fact, as expected, here's a repro https://github.com/nextjournal/clerk-table-stats/blob/739123acf3c1af086190b84cdb91ab22f27b4fea/notebooks/missing_id_repro.clj#L16-L20

Now while we could propagate the cell id at deeper children, what would that imply in the specific case of the stats-table? How does it handle nested tabular data? If we want interactive header controls at deeper levels then the sync-atom names should be a combination of the parent's cell-id and the path (see repro), but I doubt deeply nested controls make sense?

borkdude commented 4 months ago

@zampino Isn't the table viewer only really interested in top level forms?

zampino commented 4 months ago

I would say so, but the predicate matches at deeper levels. I think you can fix that if you use the :wrapped form of the predicate and argue on the count of :path from the wrapped value. Path of length 1 correspond to top-level forms.

zampino commented 4 months ago

This seems to work https://github.com/nextjournal/clerk-table-stats/compare/clerk-stats-viewer-npe?expand=1

mk commented 4 months ago

Isn't the table viewer only really interested in top level forms?

No. Tables nested inside other viewers is a use case we have and should be supported.

zampino commented 4 months ago

Ok then we‘ll propagate the cell id down to presented children and use the path to disambiguate.

borkdude commented 4 months ago

Andrea and I discussed this on a huddle yesterday and we thought nested "stats" tables might be a bit messy, so another possibility is to just render nested tables using the default table viewer rather than the stats table, but I don't have a strong opinion on this.