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

Additions to Data.Rational.* #2220

Closed
jamesmckinna opened this issue Nov 30, 2023 · 3 comments · Fixed by #2242
Closed

Additions to Data.Rational.* #2220

jamesmckinna opened this issue Nov 30, 2023 · 3 comments · Fixed by #2242

Comments

@jamesmckinna
Copy link
Contributor

Ideally, if PRs add/refactor structure on Data.Rational.Unnormalised.* then there should be a companion development for Data.Rational.* and vice versa.

cf. #1959 / #2194 (separate) and #2111 (together)

@MatthewDaggitt
Copy link
Contributor

Err, so just to play the devil's advocate. If someone makes an addition to Data.Nat, should they also be required to make the change to Data.Nat.Binary as well?

@jamesmckinna
Copy link
Contributor Author

Oh... hmmm. It's a good question!

I suppose I had in mind that Data.Rational.Unnormalised and Data.Rational are so tightly coupled. For whatever (blinkered) reason, I guess I view Rational as the 'abstract' interface implemented by the Unnormalised rationals?

But indeed, perhaps that's also true for your case, though my instinct suggests to me that these are some two free-standing, but nevertheless coupled, implementations of the naturals?

And of course we've also seen this many times in the List/Vec developments getting out of sync, and also within the various 'inflections' of the Vec hierarchy itself.

In any case, happy to close the issue with some sort of note in either HACKING or style-guide or do you think it's status:invalid?

@MatthewDaggitt
Copy link
Contributor

I guess you could add a note to HACKING along the lines of:

Before making changes to a Data module please have a look at related modules and see if they have any content along similar lines. If so then please follow those conventions (e.g. naming, argument order). For example if working on Data.Rational, please check Data.Rational.Unnormalised or if working on Data.Vec please check Data.List and vice versa.

Feel free to edit as you seefit.

@jamesmckinna jamesmckinna linked a pull request Dec 31, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants