Change annotations to use a NamedTuple #55741
Conversation
tecosaur
added
backport 1.11
|
Looks like all the tests are happy, it's just a matter of updating the docs(trings). |
tecosaur
force-pushed
the
annot-api-changes
branch
3 times, most recently
from
6a26fcb
to
1cefe5b
Compare
|
Markdown also needed to be updated. |
|
...and some more tests that need updating |
tecosaur
force-pushed
the
annot-api-changes
branch
2 times, most recently
from
5653e71
to
3409bdc
Compare
|
Hopefully that does it 🤞 |
|
CI seems to fail with some |
|
It occurs to me that we'll want to backport the StyledStrings version bump, but not the JuliaSyntaxHighlighing bump. With this in mind, I'll split the bump commit when I figure out the method ambiguity (or somebody else proposes a resolution). |
tecosaur
force-pushed
the
annot-api-changes
branch
from
3409bdc
to
db5b4ee
Compare
|
I think I've sorted out the method ambiguity that CI was complaining about, and the StyledStrings/JuliaSyntaxHighlighting bumps have been split up to make backporting easier. |
|
With tests passing, do we want to go ahead and merge this, or is there anything else I should do here? |
Backported PRs: - [x] #55773 <!-- Add compat entry for `Base.donotdelete` --> - [x] #41244 <!-- Fix shell `cd` error when working dir has been deleted --> - [x] #55795 <!-- fix #52986, regression in `@doc` of macro without REPL loaded --> - [x] #55829 <!-- [Dates] Make test more robust against non-UTC timezones --> - [x] #55641 <!-- fall back to slower stat filesize if optimized filesize fails --> - [x] #55744 <!-- fix #45494, error in ssa conversion with complex type decl --> - [x] #55783 <!-- use `inferencebarrier` instead of `invokelatest` for 1-arg `@assert` --> - [x] #55739 <!-- Add `invokelatest` barrier to `string(...)` in `@assert` --> Need manual backport: - [ ] #55798 <!-- Broadcast binary ops involving strided triangular --> Contains multiple commits, manual intervention needed: - [ ] #55509 <!-- Fix cong implementation to be properly random and not just cycling. --> - [ ] #55569 <!-- Add a docs section about loading/precomp/ttfx time tuning --> - [ ] #55824 <!-- Replace regex package module checks with actual code checks --> Non-merged PRs with backport label: - [ ] #55845 <!-- privatize annotated string API, take two --> - [ ] #55828 <!-- Fix some corner cases of `isapprox` with unsigned integers --> - [ ] #55813 <!-- Check for conflicting `@ccallable` name before JIT registration --> - [ ] #55743 <!-- doc: heap snapshot viewing --> - [ ] #55741 <!-- Change annotations to use a NamedTuple --> - [ ] #55534 <!-- Set stdlib sources as read-only during installation --> - [ ] #55499 <!-- propagate the terminal's `displaysize` to the `IOContext` used by the REPL --> - [ ] #55458 <!-- Allow for generically extracting unannotated string --> - [ ] #55457 <!-- Make AnnotateChar equality consider annotations --> - [ ] #55453 <!-- Privatise the annotations API, for StyledStrings --> - [ ] #55355 <!-- relocation: account for trailing path separator in depot paths --> - [ ] #55220 <!-- `isfile_casesensitive` fixes on Windows --> - [ ] #55169 <!-- `propertynames` for SVD respects private argument --> - [ ] #54457 <!-- Make `String(::Memory)` copy --> - [ ] #53957 <!-- tweak how filtering is done for what packages should be precompiled --> - [ ] #51479 <!-- prevent code loading from lookin in the versioned environment when building Julia --> - [ ] #50813 <!-- More doctests for Sockets and capitalization fix --> - [ ] #50157 <!-- improve docs for `@inbounds` and `Base.@propagate_inbounds` -->
|
I think this should be good to merge, but I'd appreciate it if somebody else could take a look and do so if this looks good to them. |
Backported PRs: - [x] #55849 <!-- Mmap: fix grow! for non file IOs --> - [x] #55863 <!-- Update TaskLocalRNG docstring according to #49110 --> - [x] #54433 <!-- Root globals in toplevel exprs --> - [x] #55828 <!-- Fix some corner cases of `isapprox` with unsigned integers --> - [x] #55890 <!-- Profile: fix order of fields in heapsnapshot & improve formatting --> - [x] #55884 <!-- inference: add missing `TypeVar` handling for `instanceof_tfunc` --> - [x] #55881 <!-- Install terminfo data under /usr/share/julia --> - [x] #55909 <!-- do not intentionally suppress errors in precompile script from being reported or failing the result --> - [x] #55355 <!-- relocation: account for trailing path separator in depot paths --> - [x] #55917 <!-- fix rawbigints OOB issues --> - [x] #55892 <!-- TOML: Avoid type-pirating `Base.TOML.Parser` --> - [x] #55798 <!-- Broadcast binary ops involving strided triangular --> - [x] #55919 <!-- Limit `@inbounds` to indexing in the dual-iterator branch in `copyto_unaliased!` --> Contains multiple commits, manual intervention needed: - [ ] #54009 <!-- allow extensions to trigger from packages in [deps] --> - [ ] #55509 <!-- Fix cong implementation to be properly random and not just cycling. --> - [ ] #55569 <!-- Add a docs section about loading/precomp/ttfx time tuning --> - [ ] #55824 <!-- Replace regex package module checks with actual code checks --> Non-merged PRs with backport label: - [ ] #55932 <!-- REPL: make UndefVarError aware of imported modules --> - [ ] #55910 <!-- Prevent extensions from blocking parallel pre-compilation --> - [ ] #55908 <!-- add logic to prefer loading modules that are already loaded --> - [ ] #55886 <!-- irrationals: restrict assume effects annotations to known types --> - [ ] #55871 <!-- lowering: don't reverse handler order in `(pop-handler-list ...)` --> - [ ] #55870 <!-- fix infinite recursion in `promote_type` for `Irrational` --> - [ ] #55867 <!-- update `hash` doc string: `widen` not required any more --> - [ ] #55851 <!-- [REPL] Fix #55850 by using `safe_realpath` instead of `abspath` in `projname` --> - [ ] #55813 <!-- Check for conflicting `@ccallable` name before JIT registration --> - [ ] #55743 <!-- doc: heap snapshot viewing --> - [ ] #55741 <!-- Change annotations to use a NamedTuple --> - [ ] #55534 <!-- Set stdlib sources as read-only during installation --> - [ ] #55499 <!-- propagate the terminal's `displaysize` to the `IOContext` used by the REPL --> - [ ] #55458 <!-- Allow for generically extracting unannotated string --> - [ ] #55457 <!-- Make AnnotateChar equality consider annotations --> - [ ] #55220 <!-- `isfile_casesensitive` fixes on Windows --> - [ ] #55169 <!-- `propertynames` for SVD respects private argument --> - [ ] #54457 <!-- Make `String(::Memory)` copy --> - [ ] #53957 <!-- tweak how filtering is done for what packages should be precompiled --> - [ ] #51479 <!-- prevent code loading from lookin in the versioned environment when building Julia --> - [ ] #50813 <!-- More doctests for Sockets and capitalization fix --> - [ ] #50157 <!-- improve docs for `@inbounds` and `Base.@propagate_inbounds` -->
tecosaur
force-pushed
the
annot-api-changes
branch
from
db5b4ee
to
dec2b40
Compare
|
I've just rebased + bumped the bump to incorporate JuliaLang/StyledStrings.jl#81. |
tecosaur
force-pushed
the
annot-api-changes
branch
from
dec2b40
to
f5fe438
Compare
|
Test failures seem unrelated (from Pkg) |
|
Needs a rebase. That Pkg test was fixed a few days ago |
This bump contains the following commits: a5b1174 * Adjust to change of annotations type in Base d9d7472 * Adopt Base's annotated types/functions as API da41b6a * Call load_customisations! automatically, lazily af972e0 * Fix markdown syntax in "Mark API as experimental" 8332e45 * Mark API as Experimental cfcfe8c * Fix syntax of generated CSS c82409c * Remove tag wrapping from HTML show method 03211c9 * Small optimisation to single-interpolation styled d080103 * Improve type stability within styled"" macro 448314b * More meticulously check indices in eachregion 02cd20b * Consistent Face hashes
This bump contains the following commits: 19bd57b * Adjust to change of annotations type in Base
tecosaur
force-pushed
the
annot-api-changes
branch
from
f5fe438
to
83d624c
Compare
|
Ta, there we go. I think it would be best if this could be merged in time to make it into 1.11, but I don't think Kristoffer has the bandwidth to review this and I'm generally uncomfortable with being the only person involved in approving my own PRs. |
|
Once again, test failures seem unrelated (from Pkg, but different this time). |
|
@aplavin @fatteneder since both of you were involved in the linked issues, might either of you be interested in reviewing this? |
| @@ -1,5 +1,7 @@ | |||
| # This file is a part of Julia. License is MIT: https://julialang.org/license | |||
|
|
|||
| const RegionAnnotation = NamedTuple{(:region, :label, :value), Tuple{UnitRange{Int}, Symbol, Any}} | |||
There was a problem hiding this comment.
With this definition,
julia> (region=1:2, label=:xyz, value="abc") isa RegionAnnotation
false
Maybe you want NamedTuple{(:region, :label, :value), <:Tuple{AbstractUnitRange{Int}, Symbol, Any}} instead?
Otherwise looks reasonable, don't really have any major comments.
There was a problem hiding this comment.
Thanks for giving this a look. This is mainly to avoid writing NamedTuple{(:region, :label, :value), Tuple{UnitRange{Int}, Symbol, Any}} again and again in a bunch of places, but for checking if something isa region annotation we would need to use <:Tuple{., ., <:Any}.
Maybe this should also be defined as a const for convenience? Maybe after doing so the values should be swapped?
I'm not sure, but at least I can say that this is very much a private internal (just with a general name), and so we're not locking ourselves into anything here.
Glad to hear nothing else looks notable, I didn't anticipate any major comments but I do like other people reviewing work I do in JuliaLang/julia before it's merged.
Due to popular demand, the type of annotations is to be changed from a
Tuple{UnitRange{Int}, Pair{Symbol, Any}}to aNamedTuple{(:region, :label, :value), Tuple{UnitRange{Int}, Symbol, Any}}.This requires the expected code churn to
strings/annotated.jl, and some changes to the StyledStrings and JuliaSyntaxHighlighting libraries.Closes #55249 and closes #55245.