astropy / astropy/astropy-APEs

APE to separate Quantity from representation

Open
#134 4 comments 0 reactions 0 assignees View on GitHub
Dominant language
No language data
Stars
40
Forks
41
PR merge metrics
No merged PRs in 30d

Description

Reposting from @embray https://github.com/astropy/astropy/pull/19988#issuecomment-4842879852


First, an apology: I know this thread has already consumed more of your collective time than a small papercut warrants, and now I'm expanding it further because I've got a bee in my bonnet about it. I appreciate @taldcroft's and @mhvk's considered replies. I want to explain why I think it's worth a bit more discussion, and then engage honestly with the objections you've both raised, because I think several of them are right.

### Background / why I care more than the papercut deserves

My motivation here isn't really Astropy's--it comes from the ASDF side. What sent me down this hole is that the ASDF `time-` schema is, in my opinion, tied too directly to the set of time *formats* that happen to exist in Astropy. That's historically understandable: when the schema was written, the only implementation of ASDF was in Python, and the immediate need was to round-trip Astropy `Time` objects faithfully. But it cuts against one of ASDF's foundational ideals--being a stable, language-independent archival and interchange format. The schema ends up encoding `jyear` vs `jyear_str` as if they were distinct, first-class concepts, when really they're an artifact of a specific (and quite old) API decision inside one (important) library. I'm now writing a second, independent implementation (libasdf, in C), which is exactly the situation that surfaces these leaks. It's more of an intellectual exercise in interpreting the ASDF schema as written, as if I were 100 years in the future trying to faithfully interpret the schema as documented, without reference to Astropy. So the crusade is mine, and it's aimed at the schema as much as anything. This isn't me saying Astropy did something wrong.

### On the history (for the record)

My hypothesis when I first stumbled on this was that maybe the `jyear`/`jyear_str` split predated the subformat feature (I probably already knew deep down this was wrong; it's just been a very long time since I followed the original `Time` development, much less used it). Out of curiosity I traced it: the subformat machinery (`in_subfmt`/`out_subfmt`) landed in 2012-06 (c3f18f760), and `byear_str`/`jyear_str` / `TimeEpochDateString`
came ~2 months *later*, in 2012-08 (ed24d0fd6). So the string classes don't predate subformats--they were a deliberate choice made with subformats already available. I think the reason is exactly the structural point @taldcroft raises: subformats vary representation *within* a fixed value-type, and a separate class was the only way at the time to cross the float<->string boundary. That's the crux, so let me take it head-on.

### "every format is just a representation of the underlying (JD1, JD2) time data"

This is true, but I think it flattens the very distinction I'm pointing at. There are really two independent axes hiding inside the single `format` string:

- **quantity / meaning** -- `jd` vs `mjd` vs `iso` vs `jyear`: genuinely different interpretations of the underlying time.
- **textual representation** -- bare float vs `J`-prefixed string at some precision: the *same* quantity, different glyphs.

`jd` vs `iso` differ on the first axis. `jyear` vs `jyear_str` differ *only* on the second. Treating both kinds of difference as "just another format" is the overloading I'm reacting to. The cleanest statement of my position is: a Julian year is a unit, and whether a value in that unit is printed with the unit prefix or without it is a representation question, not a different kind of time unit.

### @taldcroft's typing concern, a historical wart predating static typing

You're right that a `subfmt` that changes the *value type* breaks the property that `t.jyear` reliably infers as `float`, and that there's no precedent for it. But I'd argue the part of that which is genuinely uncomfortable is narrower than it looks: `t.jyear` and `t.jyear_str` would remain distinct, statically-typed *properties* regardless. The only thing that becomes state-dependent is `t.value`, and `t.value` is *already* dynamically typed (str for `iso`, float for `jd`, etc.). So the marginal typing loss is small, and it's really a symptom of a pre-existing design point: `format`, `out_subfmt`, and `precision` are *mutable, in-place setters* on `Time`, which already makes `t.value`'s type depend on mutable state no matter what we do about `jyear`.

So my actual proposal on this axis is: representation selection should move toward a *pure* function (method) that returns a new `Time`, rather than a mutable attribute. `Time` already has `.replicate(...)`, so the shape exists. It just doesn't fully support changing the other properies yet. Something like `t.replicate(out_subfmt="str")` (or a dedicated method) is type-safe: the returned object's `.value` type is fixed at the call site. For backwards compatibility I'd keep the `t.out_subfmt = ...` setter working, but eventually behind a deprecation warning with a deliberately long lead time; there's an enormous amount of existing code that uses it, so this can't be abrupt. It's also worth noting that while the existing interface is not type-safe, most research code doesn't really care about that.

TL;DR, the framing I'm suggesting: the mutable-setter interface predates static typing in Astropy (which is recent and still incomplete), so it was a perfectly reasonable design at the time. The setter is not type-safe and never will be but that's fine for the bulk of real-world Astropy usage, which doesn't care about typing. We just shouldn't make it the *only* path, and over time we can nudge typing-conscious code toward the pure-function form.

### @mhvk's uniqueness concern

This seems like the strongest problem technically. To clarify (IIUC) what's at stake: it's the format *guessing* code path: `Time('J2000.0')` with no explicit `format=`, where exactly one format must accept a given string for the interface to be well-defined. Today that input would only be accepted by the `jyear_str` format (and rejected by the numeric `jyear`), so the guess is unambiguous and we mustn't lose that. Apologies to @sanmaxdev for nudging in that direction in my initial "bug issue", without fully thinking through the consequences.

But let's be clear about where the ambiguity comes from: it appears only if `jyear` and `jyear_str` are kept as two separate matchable formats, *and* `jyear` is made more liberal. Then both accept `'J2000.0'` and the guessing code has two candidates. That's the approach of this PR, and I agree it's a problem. The unification I'm describing here avoids that by construction: if there's a single `jyear` quantity-format (with string-vs-float as a logically separate representation axis), then `'JXXXX.X'` resolves to exactly one format. The `J` prefix still uniquely identifies the quantity format and the representation isn't a guessing decision at all (that's `out_subfmt`, settled separately). So uniqueness is preserved by a larger reconsideration; it's the half-measure I originally proposed that breaks it.

### On "code may rely on `jyear` validating pure floats"

Technically true, and a real (if small) backwards-compat surface. If I had to guess, I'd wager that approximately nobody relies on `jyear` *rejecting* a `J`-prefixed string as a feature, but I won't pretend the chances are zero, and it's another reason any change here wants a deprecation path rather than a simple "bug fix" like I originally proposed.

### Summary

I agree with both of you that the narrow "make `jyear` liberal on input" change isn't worth it on its own, and I'm glad #19991 at least reduces my complaint to an error-message clarification (which is a genuine improvement). My remaining disagreement is only with the taxonomy: if we were designing from scratch today I think representation
would be a separate axis from quantity, and @mhvk, you've more-or-less said as much. The honest open question is whether that's worth the migration churn for a 14 year old, heavily depended on API, and I don't have a fully-formed "yes, here's why it's worth it", other than a glorified ontological nitpick.

So rather than relitigate it in a PR thread, I'm happy to write this up as an APE: a proper account of the quantity-vs-representation argument, the uniqueness-preservation story, and a conservative, long-lead-time deprecation path that keeps `.jyear_str`, `format='jyear_str'`, and the `out_subfmt` setter all working. Even if it's ultimately rejected on technically reasonable "not worth the churn" grounds, it would at least serve as durable documentation of the decision and its rationale, which I think has value in its own right. Would you be open to that?

Thanks, you can have your soapbox back.

Contributor guide

No contributing guide indexed for this repository

Research direction

No file or test is named. Start by reading the referenced PR #19988 discussion and the error-message clarification in #19991, then draft an APE covering the quantity-versus-representation argument, format-guessing uniqueness, and a conservative deprecation path; done means the proposal records these points for review.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
documentation
Issue type
Documentation
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.