Fix #1870: Rename ValueSupport subtypes for improved clarity #1872
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1872 +/- ##
=======================================
Coverage 85.99% 85.99%
=======================================
Files 144 144
Lines 8666 8666
=======================================
Hits 7452 7452
Misses 1214 1214 ☔ View full report in Codecov by Sentry. |
|
Tests are failing on Julia nightly for unrelated reasons. |
|
As mentioned in #1870, I'm not sure yet if it's worth to make this change. |
|
The names Continuous and Discrete are too inclusive in meaning. There are
other packages like DataScienceTraits.jl where the word Continuous is used
to refer to Continuous variables in general. In this package the meaning is
that of ContinuousSupport. This PR uses these more specific names.
Alternatively, I can start another PR that doesn't export Continuous nor
Discrete to avoid similar conflicts.
Em sáb., 15 de jun. de 2024, 16:39, David Widmann ***@***.***>
escreveu:
… As mentioned in #1870
<#1870>, I'm not
sure yet if it's worth to make this change.
—
Reply to this email directly, view it on GitHub
<#1872 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZQW3KOCLSISGMJ4IFGGVLZHSJ7HAVCNFSM6AAAAABJL2OUASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZQGU4TGOBXHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
It's clearly a quite general term - but IMO that's not a problem per se. I think the existence of the same (exported) name in some other package is not a sufficient reason for such a major change in Distributions that probably also causes multiple deprecation warnings in downstream packages (in particular since it seems its use in Distributions predates the one in DataScienceTraits). |
|
Is there a problem in not exporting such general names? I don't think users
of Distributions.jl rely on these. And if they do, they can be explicit
about it with an explicit import.
Em sáb., 15 de jun. de 2024, 17:51, David Widmann ***@***.***>
escreveu:
… It's clearly a quite general term - but IMO that's not a problem per se. I
think the existence of the same (exported) name in some other package is
not a sufficient reason for such a major change in Distributions that
probably also causes multiple deprecation warnings in downstream packages
(in particular since it seems its use in Distributions predates the one in
DataScienceTraits).
—
Reply to this email directly, view it on GitHub
<#1872 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZQW3PCLSMPLYQKPMJJBSTZHSSLZAVCNFSM6AAAAABJL2OUASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZQG4YDQMZTGE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
@devmotion are you willing to unexport the |
|
We can't unexport them in a non-breaking release and there are no plans for a breaking release in the near future. |
Because no one is actively leading progress in Distributions.jl or because there is a consensus among the active maintainers that breaking releases should only happen after months? |
|
Because my impression is that downstream developers are annoyed by breaking releases of such a core part of the ecosystem (IIRC there are issues with complaints and last time we did one there were also not too many positive reactions on Slack), hence we try to minimize the number of breaking releases, and there's already a list of important points to change/improve in the next breaking release. |
Isn't that always the case? Why shouldn't we fix the issue at hand in the next breaking release? It is breaking, and nothing is required downstream if we provide a deprecation warning, and document the breaking change properly. Is there a plan somewhere of the next breaking release? Can you please share it? |
I'm not against changing it in the next breaking release (I still think that it's somewhat questionable to call it a fix) but I'm opposed to making a breaking release where this is the only (?) breaking change and all the breaking changes that have been discussed (more or less publicly) for a possible future breaking release are not addressed. I also found the issue again that I had in mind: #1317 |
But I did not ask for that. What I want to know is which of the two PRs proposed is preferred for the next breaking release. This one (my preferred choice), or a new one that simply unexports the generic names? |
|
@devmotion what is the preferred solution to this problem? What else can we do to help you with the downstream demands? I checked JuliaHub, and literally 0 open-source packages make use of |
|
What is the current practice of this dichotomy? I don’t trust the docs very much, I assume it’s a mixture between the eltype of the samples and other notions of „support“ and „discrete“ and „continuous“ |
|
@mschauer I understand that it refers to the support of the CDF of the random variable. In any case, the exact meaning here shouldn't affect the issue at hand: the generic |
My preferred solution at the time being is to not do anything - there doesn't seem to be an obvious bug to me, the names have been exported for a long time without issue, the problem can be avoided easily on the user side by only loading desired functionality (e.g., using |
This PR fixes #1870.