Move TimeZones support into a package extension - #121
Merged
Conversation
TimeZones pulls in TZJData, Downloads and LibCURL, so every user of
SolarPosition paid that load cost whether or not they ever touched a
ZonedDateTime. Demote it to a weak dependency and move the zoned methods
into SolarPositionTimeZonesExt.
The relocated methods -- solar_position[!], solar_rate, calculate_deltat,
_as_utc, transit_sunrise_sunset and the next_/previous_ event helpers --
all did the same thing: convert to UTC, delegate to the DateTime method,
and convert back where the result is itself a time. Nothing in them needs
to live in the package proper.
Signatures that existed only to name ZonedDateTime are widened to an
unconstrained type parameter, so the extension's methods slot in without
the package referring to the type: TransitSunriseSunset{T}, the tspan
pair of Interpolated, _transit_sunrise_sunset_impl, and the OhMyThreads
scheduler methods.
This is invisible at the call site. A ZonedDateTime or TimeZone cannot be
constructed without TimeZones loaded, which is precisely what triggers
the extension.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Ky1A6fZaCrjV4H3Fzz2u3
Both landing pages had drifted from what the package actually ships. README: add Michalsky to the positioning table, which arrived without the table being updated, and add the new TimeZones row to the extensions table. The refraction table advertised the SPA model as `SPA`, but the exported type is `SPARefraction` -- `SPA` is the positioning algorithm, so a reader copying that name got a working call with the wrong meaning. docs index: the extensions table listed three of the five extensions, missing Interpolations as well as TimeZones. The Interpolated wrapper and the autodiff support had no mention on the page at all, though both are covered in the README and have their own guides; add a section for each, as executable examples reusing the observer the page already defines, and link the guides. Add a one-line example of passing an algorithm explicitly, since the table names algorithms the page never shows being selected. Both pages get a note that loading TimeZones is what enables ZonedDateTime arguments. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Ky1A6fZaCrjV4H3Fzz2u3
Contributor
Benchmark Results (Julia v1)Time benchmarks
Memory benchmarks
|
Contributor
Benchmark Results (Julia vlts)Time benchmarks
Memory benchmarks
|
Contributor
Benchmark Results (Julia vpre)Time benchmarks
Memory benchmarks
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #121 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 811 754 -57
=========================================
- Hits 811 754 -57 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues
There is no related issue.
Summary
TimeZoneswas a hard dependency, so every user of SolarPosition paid for it —and for
TZJData,DownloadsandLibCURLbehind it — whether or not theyever touched a
ZonedDateTime. This demotes it to a weak dependency and movesthe zoned methods into a new
SolarPositionTimeZonesExt.This is invisible at the call site. A
ZonedDateTimeorTimeZonecannot beconstructed without
TimeZonesbeing loaded, and loading it is exactly whattriggers the extension. Existing code keeps working unchanged.
What moved
The relocated methods all did the same thing — convert to UTC, delegate to the
DateTimemethod, convert back where the result is itself a time:solar_position,solar_position!(scalar + vector)Positioning/Positioning.jlsolar_rate,_as_utcPositioning/interpolated.jlcalculate_deltatPositioning/deltat.jltransit_sunrise_sunset,_transit_sunrise_sunsetUtilities/srt.jl,Utilities/spa.jlnext_/previous_sunrise, sunset, solar noonUtilities/srt.jlTransitSunriseSunset{ZonedDateTime}constructorUtilities/srt.jlSignature widening
A few signatures named
ZonedDateTimeonly to constrain a type parameter. Thoseare widened so the package no longer refers to the type, and the extension's
methods slot in:
TransitSunriseSunset{T <: Union{DateTime, ZonedDateTime}}→TransitSunriseSunset{T}Interpolated'stspan::Tuple{<:Union{DateTime, ZonedDateTime}, ...}→Tuple{Any, Any}_transit_sunrise_sunset_impl'sRandtzparametersOhMyThreadsscheduler methods'ZparameterSolarPositionOhMyThreadsExtno longer importsTimeZonesat all — it was onlyusing it to spell that constraint.
Documentation
The existing pages needed no correctness fixes for the extension change: every
page using
ZonedDateTimealready doesusing TimeZones, which is what loadsthe extension.
TimeZonesstays in[compat]and in thetest/docsprojects.While documenting the new extension, both landing pages turned out to have
drifted from what the package ships, so this also brings them current:
README
Michalskyto the positioning table — it shipped without the table being updatedTimeZonesrow to the extensions tableSPA, but the exported type isSPARefraction;SPAis the positioning algorithm, so a reader copying thatname got a call that works and means something else
docs index
Interpolationsas well as
TimeZonesInterpolatedand the autodiff support had no mention on the page at all, despiteboth being in the README and having their own guides; added a section for each as
executable
@exampleblocks reusing the observer the page already definesalgorithms the page never showed being selected
Checklist