API: allow lengths of non-scalar frames without data - #16833

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

• Do the proposed changes actually accomplish desired goals?
• Do the proposed changes follow the Astropy coding guidelines?
• Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
• Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
• Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
• Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
• Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
• Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
• At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

Just an extra note: I'll happily add text fixing the new behaviour once there is agreement this change is fine!

As far as I can tell the two methods that are being removed here served some purpose before #15121, but don't seem to be needed anymore. It would be a good idea to check if this pull request is relevant for #15415.

As far as I can tell the two methods that are being removed here served some purpose before #15121, but don't seem to be needed anymore. It would be a good idea to check if this pull request is relevant for #15415.

Yes, indeed, pre #15121 there was even more of an attempt to have the shape just be that of the data, and attributes just had to broadcast with it, while in #15121 proper broadcasting of the data was introduced. It might have been logical to change __len__ and isscalar at that time...

#15415 is not directly relevant - that is really more of an implementation detail for Attribute that only depends on shape.

The change seems reasonable to me, but it would be better if there was a test that would document that the difference between old and new behavior is intentional.

The change seems reasonable to me, but it would be better if there was a test that would document that the difference between old and new behavior is intentional.

Yes, I definitely will add tests - just want to be sure people agree the change is OK first!!

@eerovaher - I went ahead and added tests, since you seemed to think the general idea is good, and I have not yet heard from others -- @StuartLittlefair and @adrn - do comment if you think giving frame-less data a length is a bad idea.

p.s. I added the tests as an extra commit, so that one can see that before there were no tests that relied on frame-less data not having len() -- the tests passed already.

I think this change is fine. But of course this gets me thinking about discussions we have had about making frames inherently data-less, but that is not really relevant for this PR...!

Like @adrn i approve the change.

OK, great, thanks both! Since @eerovaher already said he was OK, I'll merge so that I can rebase my .info PR on it and frames can become part of tables...

p.s. I do like the idea of data-less frames only, but it will probably need a deprecation crutch where, if data is passed in, a SkyCoord is returned... Anyway, for another time...