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.

I checked the failing Check PR change log job. The towncrier action is failing because the changelog fragment number does not match this PR number:

Not all changelog file number(s) match this pull request number (19924):
docs/changes/table/3773.feature.rst

For this PR, the fragment should be renamed to:

docs/changes/table/19924.feature.rst

I also noticed there are separate red statuses from CircleCI and ReadTheDocs on the PR, so this only addresses the towncrier/changelog failure.

I checked the failing Check PR change log job. The towncrier action is failing because the changelog fragment number does not match this PR number:

Not all changelog file number(s) match this pull request number (19924):
docs/changes/table/3773.feature.rst

For this PR, the fragment should be renamed to:

docs/changes/table/19924.feature.rst

I also noticed there are separate red statuses from CircleCI and ReadTheDocs on the PR, so this only addresses the towncrier/changelog failure.

Oh thank you! Getting it done

@Omiii-215 - thanks! This is on the right track.

I have a couple of design suggestions. First is to eliminate the option of supplying a dict for the key. This does not match pandas nor the similar concept of a key in table indexing (e.g. t.loc[(1, "a")]). We can always add it later but starting the API simpler is a good thing.

Second, putting everything in a single method in the BaseGroups class is a little awkward since the keys are different. Instead lets make some private methods in the subclasses that encapsulate the differences between TableGroups and ColumnGroups. This is basically your code but refactored a bit:

# In BaseGroups:
def get_group(self, key):
    mask = self._find_group_mask(key)
    idx = np.flatnonzero(mask)
    if len(idx) == 0:
        raise KeyError(key)
    return self[idx[0]]

def _find_group_mask(self, key):
    return NotImplementedError("Subclasses must implement _find_group_mask")

# In TableGroups:
def _find_group_mask(self, key):
    """ ... docstring ... """
    keys = self.keys  # always a Table
    colnames = keys.colnames
    if not isinstance(key, (tuple, list)) and len(colnames) == 1:
        mask = keys[colnames[0]] == key
    elif isinstance(key, (tuple, list)):
        if len(key) != len(colnames):
            raise ValueError(
                f"Key tuple must have length {len(colnames)} (number of key columns)"
            )
        mask = np.ones(len(keys), dtype=bool)
        for col_name, val in zip(colnames, key):
            mask &= keys[col_name] == val
    else:
        raise ValueError("Key must be a tuple or list when grouped by multiple columns")
    return mask

# In ColumnGroups:
def _find_group_mask(self, key):
    return self.keys == key

Also, please rebase on main. In astropy we do not merge main into a feature branch. For most short-lived PRs you won't need to rebase unless a merge conflict appears.

@Omiii-215 - I pushed a couple of small commits. The one last thing this will need is an example in the narrative docs.

In the Table operations section there is the grouping docs. You can put a get_group example just after the example that starts with You can iterate over the group subtables and corresponding keys with:, showing how to access a particular group (e.g. obs_by_name.groups.get_group("M31")).

@taldcroft
Thanks! All requested changes are pushed:

• Refactored get_group: Moved it to BaseGroups and added subclass-specific _find_group_mask methods.
• Removed dict keys: get_group now only accepts tuple, list, or a single value.
• Docs added: Included a get_group example in the grouping docs.
• Cleaned history: Rebased cleanly onto main to remove the merge commit.

Note on failing CI: The mpldev failure is a known upstream issue with matplotlib's dev branch, and the RTD failure is likely from a recent numpy deprecation warning (chararray) failing the Sphinx build. Both are unrelated to these changes.

Please rebase.