Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add induceJust #202

Closed
michaelpj opened this issue May 20, 2019 · 9 comments · Fixed by #209
Closed

Add induceJust #202

michaelpj opened this issue May 20, 2019 · 9 comments · Fixed by #209

Comments

@michaelpj
Copy link

From http://hackage.haskell.org/package/witherable-0.3

Given that we can't currently have a graph with a separate key type (#141), I'm constructing a graph of keys, and then I want to map over the graph looking up the value of the key, to get a graph of values.

However, lookup is naturally a -> Maybe b, an so even with a Functor instance (#201) I'd still have a Graph (Maybe value).

Having filter/mapMaybe/catMaybes seems like an fine solution. We could either add them as simple functions or use the Filterable class.

@snowleopard
Copy link
Owner

Like Functor AdjacencyMap, this has the problem of relying on Ord constraints.

What we could do is to have a type-safe version of induce, like this:

induceJust :: Ord a => AdjacencyMap (Maybe a) -> AdjacencyMap a

Will that work for you?

@michaelpj
Copy link
Author

That would certainly work!

@snowleopard snowleopard changed the title Filterable instance for AdjacencyMap (and others?) Add induceJust May 20, 2019
@michaelpj
Copy link
Author

Also, I've realised that I can work with Graph and then only convert to AdjacencyMap later. So instance Filterable Graph would also do for me. At this point it's mostly a question of whether you want to use the class as part of the API or not.

@michaelpj
Copy link
Author

Er no, scratch that, I need to do my SCC computation before I map and induce, so I do care about AdjacencyMap.

@snowleopard
Copy link
Owner

@michaelpj Great, I'll try to add induceJust soon.

Regarding Filterable: I'd prefer to stay light on dependencies, so let's stick to induceJust for now.

@Avasil
Copy link
Contributor

Avasil commented Jun 1, 2019

Is it still relevant? I could try contributing it if nobody is working on it

@snowleopard
Copy link
Owner

@Avasil Yes, it's still relevant! Please go ahead :)

@Avasil Avasil mentioned this issue Jun 4, 2019
snowleopard pushed a commit that referenced this issue Jun 7, 2019
@snowleopard
Copy link
Owner

@michaelpj Thanks to @Avasil, we now have induceJust and induceJust1 for non-empty graphs, to be released soon.

@michaelpj
Copy link
Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants