{"action":"created","issue":{"url":"https://api.github.com/repos/pureconfig/pureconfig/issues/402","repository_url":"https://api.github.com/repos/pureconfig/pureconfig","labels_url":"https://api.github.com/repos/pureconfig/pureconfig/issues/402/labels{/name}","comments_url":"https://api.github.com/repos/pureconfig/pureconfig/issues/402/comments","events_url":"https://api.github.com/repos/pureconfig/pureconfig/issues/402/events","html_url":"https://github.com/pureconfig/pureconfig/pull/402","id":346026907,"node_id":"MDExOlB1bGxSZXF1ZXN0MjA0OTkwMDcw","number":402,"title":"Switch to parallel effects in cats instances","user":{"login":"nigredo-tori","id":5962285,"node_id":"MDQ6VXNlcjU5NjIyODU=","avatar_url":"https://avatars0.githubusercontent.com/u/5962285?v=4","gravatar_id":"","url":"https://api.github.com/users/nigredo-tori","html_url":"https://github.com/nigredo-tori","followers_url":"https://api.github.com/users/nigredo-tori/followers","following_url":"https://api.github.com/users/nigredo-tori/following{/other_user}","gists_url":"https://api.github.com/users/nigredo-tori/gists{/gist_id}","starred_url":"https://api.github.com/users/nigredo-tori/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nigredo-tori/subscriptions","organizations_url":"https://api.github.com/users/nigredo-tori/orgs","repos_url":"https://api.github.com/users/nigredo-tori/repos","events_url":"https://api.github.com/users/nigredo-tori/events{/privacy}","received_events_url":"https://api.github.com/users/nigredo-tori/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-07-31T04:03:31Z","updated_at":"2018-08-02T14:52:44Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/pureconfig/pureconfig/pulls/402","html_url":"https://github.com/pureconfig/pureconfig/pull/402","diff_url":"https://github.com/pureconfig/pureconfig/pull/402.diff","patch_url":"https://github.com/pureconfig/pureconfig/pull/402.patch"},"body":"Fixes #397.\r\n\r\nThe instances follow `cats.data.Validated` in that the errors are accumulated.\r\n\r\nTuple syntax is now supported for all three typeclasses:\r\n```scala\r\nval configReader1: ConfigReader[T1] = ...\r\n...\r\nval f: (T1, T2) => X\r\nval g: X => (T1, T2)\r\n\r\n// This was already present, but now it's parallel.\r\n(configReader1,\r\n  configReader2\r\n).mapN(f)\r\n\r\n// This is new\r\n(configWriter1,\r\n  configWriter2\r\n).contramapN(g)\r\n\r\n(configConvert1,\r\n  configConvert2\r\n).imapN(f)(g)\r\n```\r\n\r\nThe new `Semigroup[ConfigReaderFailures]` also means we can use `.parMapN` syntax for the conversion result (`Either[ConfigReaderFailures, A]`):\r\n```scala\r\n(cursor.to[T1],\r\n  cursor.to[T2]\r\n).parMapN(f)\r\n```\r\n\r\nSome issues:\r\n1. This is definitely breaking. We don't have a full `MonadError[ConfigReader]` now, and the behavior of the `ApplicativeError[ConfigReader]` has changed.\r\n2. The `ApplicativeError[ConfigReader]` instance doesn't match the monad implied by the `ConfigReader.flatMap` method. `Validated` takes the high road here by providing the monadic bind under a different name (`.andThen`), however that would probably break a lot of existing code in our case."},"comment":{"url":"https://api.github.com/repos/pureconfig/pureconfig/issues/comments/409954788","html_url":"https://github.com/pureconfig/pureconfig/pull/402#issuecomment-409954788","issue_url":"https://api.github.com/repos/pureconfig/pureconfig/issues/402","id":409954788,"node_id":"MDEyOklzc3VlQ29tbWVudDQwOTk1NDc4OA==","user":{"login":"ruippeixotog","id":613493,"node_id":"MDQ6VXNlcjYxMzQ5Mw==","avatar_url":"https://avatars3.githubusercontent.com/u/613493?v=4","gravatar_id":"","url":"https://api.github.com/users/ruippeixotog","html_url":"https://github.com/ruippeixotog","followers_url":"https://api.github.com/users/ruippeixotog/followers","following_url":"https://api.github.com/users/ruippeixotog/following{/other_user}","gists_url":"https://api.github.com/users/ruippeixotog/gists{/gist_id}","starred_url":"https://api.github.com/users/ruippeixotog/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ruippeixotog/subscriptions","organizations_url":"https://api.github.com/users/ruippeixotog/orgs","repos_url":"https://api.github.com/users/ruippeixotog/repos","events_url":"https://api.github.com/users/ruippeixotog/events{/privacy}","received_events_url":"https://api.github.com/users/ruippeixotog/received_events","type":"User","site_admin":false},"created_at":"2018-08-02T14:52:44Z","updated_at":"2018-08-02T14:52:44Z","author_association":"MEMBER","body":"Thanks for implementing this, @nigredo-tori! To be honest, I don't have enough experience with cats and pure FP type classes in general to know if providing an applicative instead of a monad is the right approach for `ConfigReader`.\r\n\r\nRegarding the first issue you mentioned, I'm not too concerned about breaking changes, but rather what we lose with this change. Can you tell me what useful operations could be done previously but can't be done now? Also, wouldn't it make sense to keep providing a non-implicit `MonadError` implementation, so that users that wanted the monad behavior could use it by importing?\r\n\r\nRegarding the second issue, I'm averse to do that change in PureConfig core right now - `flatMap` is a very common operation in the Scala library, one that non-cats users first look when they want to chain readers.\r\n"}}
{"actor":{"display_login":"ruippeixotog"}}