Add MetaDataField and hasdocstringfield - #15167

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?
• Is a milestone set? Milestone must be set but we cannot check for it on Actions; do not let the green checkmark fool you.
• 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.

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

Ping @mhvk @eerovaher for thoughts on MetaData. It's important to have a version of the descriptor that works with dataclasses.

Rebasing to pick up on fixes to dev CI job.

@mhvk, what do you think of this multi-step plan to enable MetaData to work with dataclasses?

@nstarman - sorry, anything to do with descriptors I usually need to give myself a refresher course first (and I actually wrote some fairly advanced stuff for baseband...). Combined with data classes, it becomes a lot, and then combined with frozen ones, which I hadn't really heard about at all, too much for a quickish review. Which means things never get started unless there is a really compelling improvement that comes with it...

Anyway, I now again did try to have a quick look, determined to write at least something. It mostly showed that I probably should not review, as I'm very confused... Why, e.g., is MetaDataField a dataclass itself? It does not really deal with data, just something stored elsewhere. It immediately confuses me! Especially since then the docstring stuff seems needed because it is made a data class. Or not? This does make me wonder if you are not doing too much, trying to address not just the .meta on dataclass case, but also __slots__ and frozen at the same time, without a clear case that the increased complexity is worth it. I'd try to grow with what you actually use (and perhaps have a companion PR that shows the use?).

Anyway, I think I'll have to understand the problem better too... So, a suggestion that will help elucidate that (and probably just shows that I really have no experience with data classes yet): if I just go by the problems you list up front, could one not solve the first and seemingly most important one by adjusting MetaData.__get__ itself, to check whether the owning cls is a dataclass and then adjust behaviour? The following would be exceedingly unlikely to break anything:

    def __get__(self, instance, owner):
        # When called without an instance, return self to allow access
        # to descriptor attributes.
        if instance is None:
            return None if <some test that owner is a dataclass> else self
...

Indeed, a quick test shows that I can get your non-frozen example to pass with

diff --git a/astropy/utils/metadata.py b/astropy/utils/metadata.py
index 9f4363bbae..d95b03c75f 100644
--- a/astropy/utils/metadata.py
+++ b/astropy/utils/metadata.py
@@ -532,7 +532,7 @@ class MetaData:
 
     def __get__(self, instance, owner):
         if instance is None:
-            return self
+            return self if not hasattr(owner, "__dataclass_params__") else None
         if not hasattr(instance, "_meta"):
             instance._meta = OrderedDict()
         return instance._meta
diff --git a/astropy/utils/tests/test_metadata.py b/astropy/utils/tests/test_metadata.py
index fe0b922f2b..b6bc6701e4 100644
--- a/astropy/utils/tests/test_metadata.py
+++ b/astropy/utils/tests/test_metadata.py
@@ -78,7 +78,7 @@ class TestMetaExampleData(MetaBaseTest):
 
 @dataclass
 class ExampleDataclass:
-    meta: MetaDataField = MetaDataField()  # noqa: RUF009
+    meta: MetaData = MetaData()  # noqa: RUF009
 
 
 class TestMetaExampleDataclass(MetaBaseTest):

(there must be a better way to tell whether something is a data class...)

P.S. Night here but just remembered and putting it here so it doens't go to waste: when I looked at the meta data tests, the init and set meta ones seem identical!

This does make me wonder if you are not doing too much, ... if I just go by the problems you list up front, could one not solve the first and seemingly most important one by adjusting MetaData.__get__ itself, to check whether the owning cls is a dataclass and then adjust behaviour?

That's definitely possible, I've had a number of unsubmitted PRs where I'm trying to figure out a good way to refactor
MetaData to work with dataclasses, immutable objects, and be lighter weight. I thought introducing a new class and then merging it with the old was a viable route, but I'm happy to try alternatives!

I like the idea of just modifying __get__. There's dataclasses.is_dataclass that will work for that.
In a followup PR I can add the use_obj_setter setter flag so MetaData will work on immutable (frozen) objects.

Why, e.g., is MetaDataField a dataclass itself? It does not really deal with data, just something stored elsewhere.

Any class made with dataclasses.dataclass is a "dataclass". This is confusing python terminology. I think it's to indicate that the class is defined by its data fields. @dataclasses.dataclass defines the __init__, __repr__, __eq__ etc. all based on the data fields. Also, dataclasses have very nice properties: greater introspectability (e.g. dataclass.fields), easier dynamic subclassing (e.g. dataclasses.make_dataclass), builtin IO (e.g. dataclasses.asdict).
As for MetaDataField is a dataclass itself, it's for the above reasons — to not have to manually define dunder methods and to gain all the nice properties. IMO most classes that can be migrated to dataclass (those without *args or **kwargs) should be. docs in MetaData made this a bit tricky, but many other classes in Astropy are trivially migrated. Unfortunately Cosmology is also a bit tricky because of all its descriptors, e.g. MetaData.

__slots__ and frozen at the same time, without a clear case that the increased complexity is worth it.

A goal of @dataclasses.dataclass is to make it easy to set some otherwise tricky behaviour.
slots=True (available as a flag on py3.10+) just defines __slots__, making the classes slotted and much more lightweight. frozen=True changes the behaviour of __setattr__, making attributes immutable (or as close as can be in Python: object.__setattr__ can be used to override this).

I'd try to grow with what you actually use (and perhaps have a companion PR that shows the use?).

I'd very much like to, it's in my draft PRs to make cosmology a dataclass.

OK, that all makes sense, including the final benefit of having as light-weight a class, with as few boilerplate method defintions as possible to distract one (I worry a bit about losing explicit is better than implicit especially on something like __init__, but that probably just needs a bit of getting used to it; certainly, reduction of clutter is important too!). So, happy with that goal!

My preferred path would be through small steps, on MetaData itself. So, let's start with making it work with dataclasses so you can move on. And then separate steps for using __slots__ (could be by hand at first if that makes it easy) and thus making it immutable, and converting it to a dataclass.