Intervals.jl
Intervals.jl copied to clipboard
Avoid extending Base.isopen
The isopen function defined here is very different and so it seems best we define a new function rather than extend Base.isopen. Doing this means we need to export Intervals.isopen to make this change non-breaking however doing this results in:
WARNING: both Intervals and Base export "isopen"; uses of it in module Main must be qualified
ERROR: UndefVarError: isopen not defined
It then seems best to not export Intervals.isopen and also then not export Intervals.isclosed to keep things consistent.
As this change is breaking it should be included in the next major release and we can postpone merging this until we want to make a major release.
Alternative.
We swap to using is_open and is_closed
Which follow the style guide.
and we can explort them as they don't conflict with Base.
Also we can deprecate Base.isopen and Base.isclosed to them.
Alternative. We swap to using
is_openandis_closedWhich follow the style guide. and we can explort them as they don't conflict with Base.Also we can deprecate
Base.isopenandBase.isclosedto them.
Very true we are free to name these functions however we want. That said having both is_open and isopen functions available does seem like a strange thing to do. I think keeping the names as isopen and isclosed is probably the right approach here.
That said having both is_open and isopen functions available does seem like a strange thing to do.
True, but on the other hand, Base.isopen is used so rarely that I forgot it existed.
Because normally for open things you know they are open as you openned them (and are using open(...) do.)
Thus I suspect it would be less confusing, since I think others like me also just don't see isopen being used that much.
Codecov Report
Merging #100 into master will increase coverage by
0.09%. The diff coverage is100.00%.
@@ Coverage Diff @@
## master #100 +/- ##
==========================================
+ Coverage 96.55% 96.64% +0.09%
==========================================
Files 7 7
Lines 261 328 +67
==========================================
+ Hits 252 317 +65
- Misses 9 11 +2
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/Intervals.jl | 100.00% <ø> (ø) |
|
| src/inclusivity.jl | 100.00% <100.00%> (ø) |
|
| src/interval.jl | 97.09% <100.00%> (-0.05%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 99abb65...ba7ca39. Read the comment docs.
Another example where is_open would be worse is if someone would write:
is_open(interval1) && issubset(interval1, interval2)
Seems like an unnecessary inconsistency
@oxinabox https://github.com/search?q=isopen+language%3Ajulia&type=Code
Also isopen is often used when you're not sure whether something else has closed the thing.
in our intental codebase, we use isopen exactly once from what I can tell. Though to be fair most of our IOish stuff is open source.
But this also likely means that this change to Intervals to use a different isopen is probably not going to break much, so easy upgrading