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 metadata implementation to expressions, properly freeze modint #185

Merged
merged 113 commits into from
Dec 8, 2020

Conversation

sritchie
Copy link
Member

@sritchie sritchie commented Dec 7, 2020

This PR:

  • specializes g/exp and the trig operations to square matrices (forgot to do this previously!)
  • adds a proper metadata implementation to Literal! We can use this now instead of my hand-rolled thing I was starting to do. Better to stick to Clojure's great interfaces.
  • adds a v/freeze implementation to modint, another small thing I noticed.

(withMeta [_ meta] (Literal. type expression meta))]

:cljs
[IMeta
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a circumstance where you could have Meta but not WithMeta?

Copy link
Member Author

Choose a reason for hiding this comment

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

scrolling the cljs source, it looks like you would do this for a type you don't control; the withMeta implementation would have to go onto a wrapper type, MetaFn in the case of functions with cljs.

meta on a bare function returns nil.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I was confused. I thought IWithMeta was a name you had chosen.

Are we going to maintain a difference between metadata as we see it and the native implementation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, this actually brings us back to how native works!

defrecord gets support for metadata out of the box, so that's why it worked before if it did.

But deftypes don't, so you have to manually carry around the map and implement these protocols.

(defmethod g/tanh [::matrix] [m] (series/tanh-series m))
(defmethod g/asinh [::matrix] [m] (series/asinh-series m))
(defmethod g/atanh [::matrix] [m] (series/atanh-series m))
(defmethod g/exp [::square-matrix] [m] (series/exp-series m))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I love ::square-matrix, that's a good idea.

Sam Ritchie added 25 commits December 8, 2020 05:11
@sritchie sritchie changed the base branch from sritchie/move_value_definitions to master December 8, 2020 12:50
@sritchie sritchie merged commit ee4f382 into master Dec 8, 2020
@sritchie sritchie deleted the sritchie/meta_impl_on_expressions branch December 8, 2020 13:00
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