AlgebraicJulia / Semagrams.jl

A graphical editor for graph-like structures
https://algebraicjulia.github.io/Semagrams.jl/
MIT License
91 stars 10 forks source link

Acsess refactor #147

Open sjbreiner opened 7 months ago

sjbreiner commented 7 months ago

Hi @olynch,

Here's a PR for the stuff we went over on Thursday. It's still work in progress, but I agree that it should get merged into main.

After this PR I am going to try to move to better work practices, especially merging more regularly (sorry!) and better feature separation ;-)

netlify[bot] commented 7 months ago

Deploy Preview for semagrams failed.

Name Link
Latest commit 5378fcee0623fdd90c7c9b37c0cfe17338a919ea
Latest deploy log https://app.netlify.com/sites/semagrams/deploys/658073f5809ff6000884014d
sjbreiner commented 7 months ago

I'm happy with whatever library you like. I googled "scalajs color library" and found a bunch of stuff about getting colored REPLs and 0 pointers to easy, off-the-shelf solutions, so I just wrote up my own. My only real requirement is that I should be able to use both named colors (e.g., "red") and anonymous ones (e.g., "rgb(255,0,0)" or something similar).


From: Owen Lynch @.***> Sent: Monday, November 27, 2023 12:49 AM To: AlgebraicJulia/Semagrams.jl Cc: Breiner, Spencer J. (Fed); Author Subject: Re: [AlgebraicJulia/Semagrams.jl] Acsess refactor (PR #147)

@olynch commented on this pull request.


In scala/core/src/util/attrs/Colors.scalahttps://github.com/AlgebraicJulia/Semagrams.jl/pull/147#discussion_r1405643043:

@@ -0,0 +1,223 @@ +package semagrams.util + +import upickle.default._ +import scala.language.implicitConversions + +case class RGB(red:Int,green:Int,blue:Int):

I totally agree. I like to be able to generate colors using something like LCHuv, which had a ton of research put into it to make sure that, for instance, you can sample a hue and get a uniform human-perception-adjusted brightness across sampled hues. This is what I do in GATlab.

— Reply to this email directly, view it on GitHubhttps://github.com/AlgebraicJulia/Semagrams.jl/pull/147#discussion_r1405643043, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AEO32XUNCPUB2AIK4P6HXGDYGQSX7AVCNFSM6AAAAAA7TDGM4SVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONBZGY2DGMZSGU. You are receiving this because you authored the thread.Message ID: @.***>

sjbreiner commented 7 months ago

You're going to have to run mill core.reformat on this before we merge it, of course

Can this go into the build routine on github?

olynch commented 7 months ago

Actually, running this on my computer, I'm getting a bunch of warnings about things not being able to be checked at runtime:

[warn] -- Warning: /home/o/g/AlgebraicJulia/Semagrams.jl/scala/core/src/graphs/sprites/Arrow.scala:8:41
[warn] 8 |import com.raquo.laminar.api.L.svg.{path as svgpath, _}
[warn]   |                                    ^^^^^^^^^^^^^^^
[warn]   |                                    unused import
[warn] -- Warning: /home/o/g/AlgebraicJulia/Semagrams.jl/scala/core/src/widgets/Tables.scala:67:4
[warn] 67 |    keys: Seq[Property]
[warn]    |    ^^^^
[warn]    |    unused explicit parameter
[warn] -- Unchecked Warning: /home/o/g/AlgebraicJulia/Semagrams.jl/scala/core/src/acsets/Schemas.scala:36:52
[warn] 36 |  def tables: Map[UID, Table] = elts.collect { case kv: Tuple2[UID, Table] =>
[warn]    |                                                    ^
[warn]    |the type test for (semagrams.util.UID, semagrams.Table) cannot be checked at runtime because its type arguments can't be determined from (semagrams.util.UID, semagrams.Elt)
[warn] -- Unchecked Warning: /home/o/g/AlgebraicJulia/Semagrams.jl/scala/core/src/acsets/Schemas.scala:39:50
[warn] 39 |  def fkeys: Map[UID, FKey] = elts.collect { case kv: Tuple2[UID, FKey] => kv }
[warn]    |                                                  ^
[warn]    |the type test for (semagrams.util.UID, semagrams.FKey) cannot be checked at runtime because its type arguments can't be determined from (semagrams.util.UID, semagrams.Elt)
[warn] -- Unchecked Warning: /home/o/g/AlgebraicJulia/Semagrams.jl/scala/core/src/acsets/Schemas.scala:40:53
[warn] 40 |  def attrs: Map[UID, Attr[_]] = elts.collect { case kv: Tuple2[UID, Attr[_]] =>
[warn]    |                                                     ^
[warn]    |the type test for (semagrams.util.UID, semagrams.Attr[_]) @_$2 cannot be checked at runtime because its type arguments can't be determined from (semagrams.util.UID, semagrams.Elt)
[warn] -- Unchecked Warning: /home/o/g/AlgebraicJulia/Semagrams.jl/scala/core/src/acsets/Schemas.scala:44:46
[warn] 44 |  def obs: Map[UID, Ob] = elts.collect { case kv: Tuple2[UID, Ob] => kv }
[warn]    |                                              ^
[warn]    |the type test for (semagrams.util.UID, semagrams.Ob) cannot be checked at runtime because its type arguments can't be determined from (semagrams.util.UID, semagrams.Elt)
[warn] -- Unchecked Warning: /home/o/g/AlgebraicJulia/Semagrams.jl/scala/core/src/acsets/Schemas.scala:46:9
[warn] 46 |    case kv: Tuple2[UID, Hom[_, _]] => kv
[warn]    |         ^
[warn]    |the type test for (semagrams.util.UID, semagrams.Hom[_, _²]) @_$4 @_$3 cannot be checked at runtime because its type arguments can't be determined from (semagrams.util.UID, semagrams.Elt)
[warn]    |
[warn]    |where:    _  is a type in method isDefinedAt with bounds <: semagrams.Ob
[warn]    |          _² is a type in method isDefinedAt with bounds <: semagrams.Ob
[warn] -- Unchecked Warning: /home/o/g/AlgebraicJulia/Semagrams.jl/scala/core/src/rendering/Sources.scala:213:35
[warn] 213 |      f <- s.homSeq.collect { case f: PartHom => f }
[warn]     |                                   ^
[warn]     |the type test for semagrams.PartHom cannot be checked at runtime because it's a refinement type
[warn] -- Unchecked Warning: /home/o/g/AlgebraicJulia/Semagrams.jl/scala/core/src/rendering/Sources.scala:214:35
[warn] 214 |      g <- s.homSeq.collect { case f: PartHom => f }
[warn]     |                                   ^
[warn]     |the type test for semagrams.PartHom cannot be checked at runtime because it's a refinement type
[warn] -- Unchecked Warning: /home/o/g/AlgebraicJulia/Semagrams.jl/scala/core/src/rendering/Sources.scala:299:22
[warn] 299 |      .collect { case f: PartHom => f }
[warn]     |                      ^
[warn]     |the type test for semagrams.PartHom cannot be checked at runtime because it's a refinement type
[warn] 10 warnings found

@sjbreiner I feel like this might mess things up subtly; do you know what you are doing?

olynch commented 7 months ago

OK, I'm also noticing that the

case Event.MouseLeave(backgrounPart) => IO(Some(()))

branch of the match statement in MoveViaDrag is firing when I leave the box part. So perhaps something is broken about == for parts?

olynch commented 7 months ago

Another regression; you don't make the arrow unhoverable while you drag, so it intercepts the hover event that you want to send to the thing you are dragging the arrow to.

olynch commented 7 months ago

There are also a ton of printlns everywhere...

olynch commented 7 months ago

OK, in general the whole "one part for two things" stuff that's going on for Homs in the acset editor is super buggy. I don't really know how it is supposed to work, but I think that it breaks a ton of implicit assumptions elsewhere in the code, like in the hover code.

sjbreiner commented 7 months ago

OK, I'm also noticing that the

case Event.MouseLeave(backgrounPart) => IO(Some(()))

branch of the match statement in MoveViaDrag is firing when I leave the box part. So perhaps something is broken about == for parts?

I'm not seeing this. I've commented it back in, b/c otherwise the drag continues away from the svg element and can miss the mouseup

sjbreiner commented 7 months ago

Actually, running this on my computer, I'm getting a bunch of warnings about things not being able to be checked at runtime: ... @sjbreiner I feel like this might mess things up subtly; do you know what you are doing?

These are the match issues that I was talking about. I think I've resolved these by replacing match kv: Tuple2[A,B] => ... by match (a:A,b:B) => ...

sjbreiner commented 7 months ago

There are also a ton of printlns everywhere...

Removed

sjbreiner commented 7 months ago

OK, in general the whole "one part for two things" stuff that's going on for Homs in the acset editor is super buggy. I don't really know how it is supposed to work, but I think that it breaks a ton of implicit assumptions elsewhere in the code, like in the hover code.

As I mentioned, the rendering pipeline needed to be converted to use keys that are more informative than parts. I've done that and I think it's addressed most of the problems that you were running into above.

olynch commented 7 months ago

OK, I'm also noticing that the

case Event.MouseLeave(backgrounPart) => IO(Some(()))

branch of the match statement in MoveViaDrag is firing when I leave the box part. So perhaps something is broken about == for parts?

I'm not seeing this. I've commented it back in, b/c otherwise the drag continues away from the svg element and can miss the mouseup

Seems to be fixed now...