move definition of Domain to DomainSetsCore package#170
move definition of Domain to DomainSetsCore package#170daanhb wants to merge 1 commit intoJuliaMath:masterfrom
Conversation
|
The build fails because DomainSetsCore hasn't been registered yet. All tests pass locally for me. |
hyrodium
left a comment
There was a problem hiding this comment.
All changes in this PR look good to me.
domaineltype(::AbstractInterval) will be added in another PR, right? (#150 (comment))
|
To be sure, in the current version the DomainSetsCore package does the following:
The |
|
Hence, if IntervalSets would switch to using DomainSetsCore right now, the |
|
Having reread a few issues (thanks for the link to gentype, interesting related discussion): moving the definition of Domain to a new package and settling the |
|
Thank you for the quick detailed response! I totally agree that IntervalSets.jl should not own ProposalReconsidered a little, I think we have the following choices A, B, and C. A
# Defined in IntervalSets.jl
abstract type AbstractInterval{T} <: DomainSetsCore.Domain end
DomainSetsCore.domaineltype(::Interval{L,R,T}) where {L,R,T} = float(T)
B
# Defined in IntervalSets.jl
abstract type AbstractInterval{T} end
# Defined in some package extension
DomainSetsCore.domaineltype(::IntervalSets.Interval{L,R,T}) where {L,R,T} = float(T)
DomainSetsCore.DomainStyle(::IntervalSets.AbstractInterval) = IsDomain()IntervalSets.jl do not care the C
# Defined in IntervalSets.jl
abstract type AbstractInterval{T} <: DomainSetsCore.Domain{T} endThe function ConclusionI am not particularly sure which option, A, B, or C, is the best.
Sorry if I missed anything in our past discussions so far. |
|
Thanks for the overview, that helps to clarify the issues. I agree that these are possibilities, with pros and cons to each, but for the sake of completeness I think there are at least two more: DIntervals have two type parameters: EWe don't change anything and just move FLike B, but Some further observations and perspectives
|
|
Looking at options:
Currently, option C seems most viable to me. We can remove the definition of |
|
To add, I'm not in favour of any solution in which |
|
For the sake of the argument, I disagree with the idea that there is no iteration involved in the definition of intervals (and hence no link with For example: julia> using DomainIntegrals, IntervalSets
julia> integral(exp(x) for x in 0..1)
1.7182818284590453
julia> integral(exp(x) for x in 0..big(1))
1.718281828459045235360287471352662497757247093699959574966967627724076630353935This notation defines the function |
|
I thought the plan was to move to more of an interface than an abstract type? Why not use this as an opportunity to just delete The interface could be |
Let's not discuss that here. That is a design question for the new core package. |
|
Okay to register DomainSetsCore.jl? Since the discussion here I've removed |
|
FYI, we're preparing a breaking update of DomainSets. With some delay... :-) It might be an idea to proceed? It could go like this:
Meanwhile, I'm exploring how to change DomainSets such that no type piracy is left. Depending on how that goes it can go into v0.8, or into a later v0.9. I think everyone involved was mostly fine with that, perhaps up to a name change to ContinuousSetsCore.jl or something like that. The main alternative is for IntervalSets to just remove the Domain{T} type. That is mainly a breaking change for DomainSets, not here, for which I do expect some fallback but would still rather do that before v1 of either package. |
This is an up-to-date pull request for the possible move of Domain{T} to DomainSetsCore.jl, see #169
I did not move these two lines:
as I'm not sure we should have these at the generic level of
Domainin DomainSetsCore. For compatibility within this package, I kept them for intervals: