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

Resolves issue #509 and resolves issue #626 #1822

Merged
merged 26 commits into from
Sep 29, 2022

Conversation

jamesmckinna
Copy link
Contributor

@jamesmckinna jamesmckinna commented Sep 15, 2022

First things first: resolving issue #626 for Data.List.Properties

  • changed 'relative' properties to -local
  • this needs to be noted in CHANGELOG

Secondly, resolving issue #509 going through and making map-∘ properties etc. have consistent names across the library

  • Data.List.Properties
  • what/where else? see below, esp. as regards Codata.*

@jamesmckinna
Copy link
Contributor Author

jamesmckinna commented Sep 15, 2022

Additionally,

  • Data.List.NonEmpty.Properties
  • Data.List.Zipper.Properties
  • Data.List.Relation.Binary.Suffix.Heterogeneous.Properties
  • Data.Tree.Rose.Properties
  • Data.Fin.Substitution.List
  • Data.Maybe.Properties

@jamesmckinna
Copy link
Contributor Author

jamesmckinna commented Sep 15, 2022

And what do we do about these?
The relevant property uses the opposite direction of the bisimilarity relation, because map-map-fusion is meant to simplify/create opportunities for simplification, by pushing the function composition inwards. So do we rename to map-∘ or keep as is?

  • Codata.Guarded.Stream.Properties
  • Codata.Sized.M.Properties
  • Codata.Sized.Delay.Properties
  • Codata.Sized.Covec.Properties
  • Codata.Sized.Colist.Properties
  • Codata.Sized.Stream.Properties

That said, for consistency's sake, we could nevertheless rename map-identity in each to map-id.
EDITED: How to proceed? map-map-fusion and map-identity now both deprecated in favour of the 'standard' names.

@JacquesCarette
Copy link
Contributor

You might want to tackle this more incrementally (i.e. don't do map-id changes in this PR).

My impression is that changes like these do show up in CHANGELOG.

Don't know about Codata.

@jamesmckinna
Copy link
Contributor Author

Re: incremental changes
as in my comment on the (in my mind, at least) coupled issues, I decided to roll the changes up together in a single PR.
Changing map-identity to map-id is arguably a separate change, but theres' no knock-on viscosity in std-lib, so only clients' code need pay attention to the deprecation... and it's still only a deprecation warning; ergo, harmless?

@JacquesCarette
Copy link
Contributor

The benefit of small PRs is that if there's a problem with one thing, most of the pieces can still be merged in, while if it's all in one big one, perfection must be attained before progress happens.

@jamesmckinna jamesmckinna changed the title Issues509+626 Resolves issue #509 and resolves issue #626 Sep 16, 2022
@jamesmckinna jamesmckinna linked an issue Sep 16, 2022 that may be closed by this pull request
@gallais
Copy link
Member

gallais commented Sep 16, 2022

map-map-fusion (...) do we rename to map-∘

I think we should. For uniformity we may want to flip the lemma too but I don't know whether that's pushing it too far.
Then again 2.0 is the right moment to have these kind of breaking changes.

@jamesmckinna
Copy link
Contributor Author

Thanks @gallais for the prompt about Codata.*. Similarly change map-identity to map-id? But I think at this stage I would not change the order of the equalities. But it's probably tomorrow or Monday now before I'll tackle these in any case...

@jamesmckinna
Copy link
Contributor Author

I propose marking this as status:ready, while noting that resolving #509 might also entail another renaming to resolve #501.

@jamesmckinna jamesmckinna added this to the v2.0 milestone Sep 17, 2022
@MatthewDaggitt
Copy link
Contributor

Looks great, thanks for the PR @jamesmckinna !

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.

Use -local consistently for properties
4 participants