Skip to content

Add padRight and truncate to Data.Vec, and prove some properties #1640

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

Merged
merged 8 commits into from
Feb 17, 2022

Conversation

TOTBWF
Copy link
Contributor

@TOTBWF TOTBWF commented Dec 4, 2021

Patch Description

This PR adds the following functions to Data.Vec.Base

truncate : ∀ {m n} → m ≤ n → Vec A n → Vec A m
pad : ∀ {m n} → m ≤ n → A → Vec A m → Vec A n

It also proves some properties of these two functions.

@gallais
Copy link
Member

gallais commented Dec 4, 2021

In Bounded we have:

padRight : ∀ {n} → A → Vec≤ A n → Vec A n
padLeft : ∀ {n} → A → Vec≤ A n → Vec A n
padBoth : ∀ {n} → A → A → Vec≤ A n → Vec A n

so we should probably stick to that naming convention.

@TOTBWF
Copy link
Contributor Author

TOTBWF commented Dec 4, 2021

Yeah, that seems reasonable! Should we mark this as a breaking change then? If you import Data.Vec.Bounded and Data.Vec, you could get conflicts.

@TOTBWF TOTBWF changed the title Add pad and truncate to Data.Vec, and prove some properties Add padRight and truncate to Data.Vec, and prove some properties Dec 4, 2021
@MatthewDaggitt MatthewDaggitt added this to the v2.0 milestone Dec 5, 2021
Copy link
Contributor

@MatthewDaggitt MatthewDaggitt left a comment

Choose a reason for hiding this comment

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

Thanks for the great PR! A few minor cosmetic comments.

@MatthewDaggitt
Copy link
Contributor

Should we mark this as a breaking change then? If you import Data.Vec.Bounded and Data.Vec, you could get conflicts.

It's a good thought, but we add dozens of duplicated function names throughout the library every release so we tend not to. Also recent versions of Agda tends to give reasonable error messages for clashes so it's not too hard to debug the problem.

@MatthewDaggitt
Copy link
Contributor

@TOTBWF any chance you have the time to make the minor changes from the reviews and then I'll merge this in?

@gallais gallais self-assigned this Feb 16, 2022
@TOTBWF
Copy link
Contributor Author

TOTBWF commented Feb 16, 2022

Sorry, I've been quite busy recently and this fell off my radar!

@MatthewDaggitt
Copy link
Contributor

Thanks @gallais!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants