Skip to content

The modules in semialign have a lot of overlapping exports #198

New issue
New issue
Open
Open
The modules in semialign have a lot of overlapping exports#198

Description

@mitchellwrosen
mitchellwrosen
opened on May 30, 2024

Hi there,

I've noticed that the semialign package exports many symbols from more than one module.

These symbols are exported by both Data.Align and Data.Semialign:

  • Align(..)
  • Unalign(..)
  • alignVectorWith
  • lpadZip
  • lpadZipWith
  • padZip
  • padZipWith
  • rpadZip
  • rpadZipWith
  • salign

These symbols are exported by both Data.Semialign and Data.Zip:

  • Unzip(..)
  • Zip(..)
  • unzipDefault

And finally, these symbols are exported by Data.Align, Data.Semialign, and Data.Zip:

  • Semialign(..)

I would not have ever noticed maybe 5 years ago, but now HLS is robust enough to recommend imports when a symbol is out of scope. So, I think it would be preferred if each symbol was only exported by one module.

Activity

  1. phadej commented on May 31, 2024

    @phadej
    phadej
    on May 31, 2024
    Collaborator

    In the beginning there was only Data.Align with only Align type-class. Then hierarchy was refactored and all things were put into Data.Semialign, but the Data.Align is there for backward-compat (and Data.Zip for consistency).

    I'm not really keen to just remove exports, as that would be breaking change for no reason. If there will be another breaking change, than maybe then would make sense to bundle.

    So, I think it would be preferred if each symbol was only exported by one module.

    Say that to base (and ghc-internal) ;) or about any package having exposed internal modules.

  2. mitchellwrosen commented on May 31, 2024

    @mitchellwrosen
    mitchellwrosen
    on May 31, 2024
    Author

    Sure, I agree with the backwards compatibility concerns. The problem is certainly exacerbated by HLS providing a plethora of similar options.

    Well, this is where we're at today (just to put a visual on the ticket):

    2024-05-31-100621_789x436_scrot

  3. phadej commented on May 31, 2024

    @phadej
    phadej
    on May 31, 2024
    Collaborator

    The way HLS suggests four options per module is HLS's issue.

  4. rickowens commented on Jul 16, 2024

    @rickowens
    rickowens
    on Jul 16, 2024

    as that would be breaking change for no reason

    I would like to politely object to the "no reason" characterization. I believe cleaning up the API is a noble cause, highly valuable for its own sake! As always, this must be weight against all factors, but IMO a clean API is significantly more important than "no reason".

    edit: I am also aware that the problem is widespread, including in base. base also uses String everywhere when modern libraries use Text :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions