-
Notifications
You must be signed in to change notification settings - Fork 27
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
Stop encouraging type-piracy; suggest "dynamic" vendoring #56
Conversation
You're just trying to annoy me wrt Revise, right? 😄 More seriously, this is probably a good thing to add. I've done something similar with https://github.com/JuliaArrays/CustomUnitRanges.jl. It's a bit problematic wrt two packages that need to both manipulate intervals, so I am not sure I'd phrase this quite as strongly as you have. At least to me, there are cases where the IMO, piracy is not that different from starting Julia with different |
Oops, that was not my intention. But I thought it'd work with Revise because Revise can handle
Yes, if we were to add this section in the README, I think this point should be emphasized. I included one sentence but it can be explained in more details.
I'm OK with phrasing it differently. Maybe a simple fix would be to change the sentence to "... to avoid unnecessary type-piracy." (add "unnecessary").
I think cases like OffsetArrays make sense but users need to trust that package authors have good taste. It also would be problematic if two packages pirated IntervalSets.jl in an incompatible way. This is why I prefer IntervalSets.jl to dis/encourage type-piracy as the same level as Base.
Off topic, but I think it'd be nice if Pkg.jl helps doing type-piracy in more controlled manner. Some metadata fields like |
That was mostly a joke. But I am not quite certain what happens if the exact same file gets included by more than one package. Revise's internal structure may make the (unfortunate) assumption that one file gets included by one package. If it's a problem it's surely fixable, perhaps with some effort.
I had the same thought. There might be ways of getting the same behavior as In the meantime, I'd be perfectly happy merging a version of this that is a bit more nuanced in terms of its recommendations. And as necessary I will fix Revise 😄. |
It would be good to understand the concrete examples where type piracy is causing issues. As of now it feels like solving a hypothetical problem with a solution that’s likely to cause non-hypothetical problems... |
@dlfivefifty A (somewhat?) concrete problem would be defining But I think potential problems are enough to avoid encouraging type-piracy. I had to think very carefully if I wanted to use IntervalSets.jl in my code because of that sentence. A "Base" library like IntervalSets.jl becomes powerful if it is used in many packages. So, I think it's better to get rid of the psychological blocker even if it is due to potential problems, to help people depending on this package. Edit: non-monotonic -> discontinuous |
@timholy I opened an issue and replied to your comment in JuliaLang/Pkg.jl#1390
I see. It make sense that it can happen.
Thank you very much in advance! |
@timholy Is this comment #55 (comment) rather talking about this issue? Assuming that's the case, what do you think about my comment above #56 (comment)? I can probably come up with a few more examples with similar quality. But, if you consider an example is valid only if it already occurred, it's rather pointless to list them all. (Note to myself: another example might be scalar |
In the case of conflicting definitions, won't you get a warning when you load the second package? But I guess you're saying you want the functionality of both packages within their own domain. Yes, I suppose that's reasonable. (I don't know why you'd want to round inside, but I'm sure there must be circumstances...) |
This PR is stale and #149 updates the README, so I'll close this PR. |
I find it a bit uneasy that IntervalSets.jl encourages type-piracy. I think it might be preventing people from using it as a critical dependency. Instead, I suggest pointing people to use what I call "dynamic" vendoring (npm style import?). The idea is to use
instead of
using IntervalSets
so that the package "owns" the types and functions defined in the submodule.IntervalSets
.See more explanation in the README I edited. You can read the rendered version here: https://github.com/tkf/IntervalSets.jl/tree/vendoring#including-intervalsetsjl-as-a-submodule