{"action":"created","issue":{"url":"https://api.github.com/repos/gap-system/gap/issues/147","labels_url":"https://api.github.com/repos/gap-system/gap/issues/147/labels{/name}","comments_url":"https://api.github.com/repos/gap-system/gap/issues/147/comments","events_url":"https://api.github.com/repos/gap-system/gap/issues/147/events","html_url":"https://github.com/gap-system/gap/pull/147","id":65506007,"number":147,"title":"Cleanup ZmodnZ code","user":{"login":"fingolfin","id":241512,"avatar_url":"https://avatars.githubusercontent.com/u/241512?v=3","gravatar_id":"","url":"https://api.github.com/users/fingolfin","html_url":"https://github.com/fingolfin","followers_url":"https://api.github.com/users/fingolfin/followers","following_url":"https://api.github.com/users/fingolfin/following{/other_user}","gists_url":"https://api.github.com/users/fingolfin/gists{/gist_id}","starred_url":"https://api.github.com/users/fingolfin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fingolfin/subscriptions","organizations_url":"https://api.github.com/users/fingolfin/orgs","repos_url":"https://api.github.com/users/fingolfin/repos","events_url":"https://api.github.com/users/fingolfin/events{/privacy}","received_events_url":"https://api.github.com/users/fingolfin/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/gap-system/gap/labels/enhancement","name":"enhancement","color":"84b6eb"}],"state":"closed","locked":false,"assignee":{"login":"alex-konovalov","id":5751387,"avatar_url":"https://avatars.githubusercontent.com/u/5751387?v=3","gravatar_id":"","url":"https://api.github.com/users/alex-konovalov","html_url":"https://github.com/alex-konovalov","followers_url":"https://api.github.com/users/alex-konovalov/followers","following_url":"https://api.github.com/users/alex-konovalov/following{/other_user}","gists_url":"https://api.github.com/users/alex-konovalov/gists{/gist_id}","starred_url":"https://api.github.com/users/alex-konovalov/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alex-konovalov/subscriptions","organizations_url":"https://api.github.com/users/alex-konovalov/orgs","repos_url":"https://api.github.com/users/alex-konovalov/repos","events_url":"https://api.github.com/users/alex-konovalov/events{/privacy}","received_events_url":"https://api.github.com/users/alex-konovalov/received_events","type":"User","site_admin":false},"milestone":null,"comments":10,"created_at":"2015-03-31T17:37:41Z","updated_at":"2015-07-16T11:27:44Z","closed_at":"2015-04-22T08:27:16Z","pull_request":{"url":"https://api.github.com/repos/gap-system/gap/pulls/147","html_url":"https://github.com/gap-system/gap/pull/147","diff_url":"https://github.com/gap-system/gap/pull/147.diff","patch_url":"https://github.com/gap-system/gap/pull/147.patch"},"body":"This cleans up the ZmodnZ code a bit. In particular, we used to store the modulus `n` in three places: \r\n\r\n1. in the type(s) via `DataType`,\r\n2. in the family as `fam!.modulus`, and\r\n3. in the family in the attribute `Characteristic(fam)`.\r\n\r\nThe last commit in this PR unifies this to the third variant. For now, the first variant is also retained (i.e. we still store the modulus in the type) becaues the Browse package accesses it there, like this:\r\n```String( DataType( TypeObj( m[1][1] ) ) ) )```\r\nThis code should be changed to\r\n```String( Characteristic( m[1][1] ) ) )```\r\nor\r\n```String( ModulusOfZmodnZObj( m[1][1] ) ) )```\r\nthen it'll work with both old and new GAP versions, and then we could get rid of this usage of DataType. \r\n\r\nThere is only one other use of `DataType` in the GAP library: Generalized mapping use it store source and range; they do this because the underlying representation / implementation could be a positional object, so we cannot rely on attribute storage.\r\n\r\nOutside the library, the `cvec` package is the only other user I am aware of. I am hopeful it could be refactored to not use `DataType`, too, though, but I did not look closely."},"comment":{"url":"https://api.github.com/repos/gap-system/gap/issues/comments/121930163","html_url":"https://github.com/gap-system/gap/pull/147#issuecomment-121930163","issue_url":"https://api.github.com/repos/gap-system/gap/issues/147","id":121930163,"user":{"login":"fingolfin","id":241512,"avatar_url":"https://avatars.githubusercontent.com/u/241512?v=3","gravatar_id":"","url":"https://api.github.com/users/fingolfin","html_url":"https://github.com/fingolfin","followers_url":"https://api.github.com/users/fingolfin/followers","following_url":"https://api.github.com/users/fingolfin/following{/other_user}","gists_url":"https://api.github.com/users/fingolfin/gists{/gist_id}","starred_url":"https://api.github.com/users/fingolfin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fingolfin/subscriptions","organizations_url":"https://api.github.com/users/fingolfin/orgs","repos_url":"https://api.github.com/users/fingolfin/repos","events_url":"https://api.github.com/users/fingolfin/events{/privacy}","received_events_url":"https://api.github.com/users/fingolfin/received_events","type":"User","site_admin":false},"created_at":"2015-07-16T11:27:44Z","updated_at":"2015-07-16T11:27:44Z","body":"@alex-konovalov Well, strictly speaking OpenMath was relying on undocumented behavior, so I'd say this is a bug in it... Indeed, I'd also consider using `x![1]` a bug -- it relies on a specific representation. Better would be to use `Int(x)` or `ExtRepOfObj(x)`.\r\n\r\nThat said, of course I do not want to break existing packages (which is why I left DataType support in  for now, to support the `Browse` package). But to stay compatible with the old OpenMath, we'd have to add the DataType back in, which would be kind of sad... So, Alex, if you could make a new release of OpenMath with this changed, that would be awesome :)"}}