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

Haskell-style map function for ACSets #352

Merged
merged 4 commits into from Dec 21, 2020

Conversation

olynch
Copy link
Contributor

@olynch olynch commented Dec 7, 2020

Pretty simple, basically:

map(Int,s -> Int(s[1]),:Weight,g::WeightedGraph{String})::WeightedGraph{Int}

All attributes with a given codomain get converted using the supplied function. Partly addresses AlgebraicJulia/ACSets.jl#7.

@olynch
Copy link
Contributor Author

olynch commented Dec 10, 2020

@epatters btw this is ready to be reviewed.

@epatters
Copy link
Member

Cool, thanks. I'll review it soon.

@epatters
Copy link
Member

epatters commented Dec 11, 2020

Thanks Owen. Two questions/comments:

  1. Do you think there will be many use cases for this as is? My worry is that if you want to transform multiple attributes, you'd have to call map multiple times and end up reallocating the whole C-set each time. Also, maybe sometimes you want to map all data attributes with given codomain, as here, or just a particular data attribute. I think you could generalize this to something like mutate in dplyr without too much change to the structure of your code but a big boost in usefulness.

  2. In idiomatic Julia you shouldn't usually have to specify types that can be inferred. So instead of supplying Int as a first argument, you compute map(s -> Int(s[1]), ...) in base Julia, which returns a Vector{Int} whose element type you can extract. I guess this would complicate the code a bit.

Putting these suggestions together, I would imagine an API supporting examples like

map(g::WeightedGraph{Symbol}, weight = s -> Int(s[1])) # `weight` attribute only
map(g::WeightedGraph{Symbol}, Weight = s -> Int(s[1])) # all attributes with codomain `Weight`

map(g::RelationDiagram{Symbol}, name = titlecasestring, variable = uppercasestring)

What do you think?

@olynch
Copy link
Contributor Author

olynch commented Dec 14, 2020

OK, so I guess then there is an error if you don't correctly map all of the attributes to a new type?

Yeah, can do!

@epatters epatters mentioned this pull request Dec 14, 2020
@olynch olynch requested a review from epatters December 19, 2020 13:15
Copy link
Member

@epatters epatters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Owen, I think this method will be very useful.

src/categorical_algebra/CSetDataStructures.jl Outdated Show resolved Hide resolved
src/categorical_algebra/CSetDataStructures.jl Outdated Show resolved Hide resolved
_map(acs, fns)
end

function sortunique!(x)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice if this were built into Julia. I call unique!(sort!(x)) all over the codebase.

@epatters
Copy link
Member

Awesome, merging now.

@epatters epatters merged commit 7542248 into AlgebraicJulia:master Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants