Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Eero Vaher <eero.vaher@gmail.com>
  • Loading branch information
nstarman and eerovaher committed Jun 4, 2023
1 parent 0d9391a commit d57f2db
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 18 deletions.
6 changes: 3 additions & 3 deletions astropy/cosmology/io/ecsv.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def read_ecsv(
will be ``{'key': 10}``).
rename : dict or None (optional keyword-only)
A dictionary mapping column names to fields of the Cosmology.
A dictionary mapping column names to fields of the `~astropy.cosmology.Cosmology`.
**kwargs
Passed to :attr:`astropy.table.QTable.read`
Expand Down Expand Up @@ -71,7 +71,7 @@ def write_ecsv(
Parameters
----------
cosmology : |Cosmology|
cosmology : `~astropy.cosmology.Cosmology`
The cosmology instance to convert to a mapping.
file : path-like or file-like
Location to save the serialized cosmology.
Expand All @@ -85,7 +85,7 @@ def write_ecsv(
Whether to put the cosmology class in the Table metadata (if `True`,
default) or as the first column (if `False`).
rename : dict or None (optional keyword-only)
A dictionary mapping fields of the Cosmology to columns of the table.
A dictionary mapping fields of the `~astropy.cosmology.Cosmology` to columns of the table.
**kwargs
Passed to ``cls.write``
Expand Down
3 changes: 2 additions & 1 deletion astropy/cosmology/io/mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ class itself.
filling in any non-mandatory arguments missing in 'map'.
rename : dict or None (optional, keyword-only)
A dictionary mapping keys in 'map' to fields of the Cosmology.
A dictionary mapping keys in ``map`` to fields of the
`~astropy.cosmology.Cosmology`.
Returns
-------
Expand Down
12 changes: 6 additions & 6 deletions astropy/cosmology/io/row.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@


def from_row(row, *, move_to_meta=False, cosmology=None, rename=None):
"""Instantiate a |Cosmology| from a `~astropy.table.Row`.
"""Instantiate a `~astropy.cosmology.Cosmology` from a `~astropy.table.Row`.
Parameters
----------
Expand All @@ -32,11 +32,11 @@ def from_row(row, *, move_to_meta=False, cosmology=None, rename=None):
filling in any non-mandatory arguments missing in 'table'.
rename : dict or None (optional, keyword-only)
A dictionary mapping columns in the row to fields of the Cosmology.
A dictionary mapping columns in the row to fields of the `~astropy.cosmology.Cosmology`.
Returns
-------
|Cosmology| subclass instance
`~astropy.cosmology.Cosmology`
Examples
--------
Expand Down Expand Up @@ -68,7 +68,7 @@ def from_row(row, *, move_to_meta=False, cosmology=None, rename=None):
kcosmo = inv_rename.get("cosmology", "cosmology")

# special values
name = row[kname] if kname in row.columns else None # get name from column
name = row.get(kname)

meta = defaultdict(dict, copy.deepcopy(row.meta))
# Now need to add the Columnar metadata. This is only available on the
Expand All @@ -95,7 +95,7 @@ def to_row(cosmology, *args, cosmology_in_meta=False, table_cls=QTable, rename=N
Parameters
----------
cosmology : |Cosmology|
cosmology : `~astropy.cosmology.Cosmology`
The cosmology instance to convert to a mapping.
*args
Not used. Needed for compatibility with
Expand All @@ -116,7 +116,7 @@ def to_row(cosmology, *args, cosmology_in_meta=False, table_cls=QTable, rename=N
Examples
--------
A Cosmology as a `~astropy.table.Row` will have the cosmology's name and parameters
A `~astropy.cosmology.Cosmology` as a `~astropy.table.Row` will have the cosmology's name and parameters
as columns.
>>> from astropy.cosmology import Planck18
Expand Down
15 changes: 9 additions & 6 deletions astropy/cosmology/io/table.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,17 @@ def from_table(table, index=None, *, move_to_meta=False, cosmology=None, rename=
(e.g. for ``Cosmology(meta={'key':10}, key=42)``, the ``Cosmology.meta``
will be ``{'key': 10}``).
cosmology : str or |Cosmology| class or None (optional, keyword-only)
cosmology : str or type or None (optional, keyword-only)
The cosmology class (or string name thereof) to use when constructing
the cosmology instance. The class also provides default parameter values,
filling in any non-mandatory arguments missing in 'table'.
rename : dict or None (optional, keyword-only)
A dictionary mapping columns in 'table' to fields of the Cosmology class.
A dictionary mapping columns in 'table' to fields of the `~astropy.cosmology.Cosmology` class.
Returns
-------
|Cosmology| subclass instance
`~astropy.cosmology.Cosmology`
Examples
--------
Expand Down Expand Up @@ -115,7 +115,10 @@ def from_table(table, index=None, *, move_to_meta=False, cosmology=None, rename=
if not table.indices: # no indexing column, find by string match
nc = "name" # default name column
if rename is not None: # from inverted `rename`
nc = {v: k for k, v in rename.items()}.get(nc, nc)
for key, value in rename.items():
if value == "name":
nc = key
break

indices = np.where(table[nc] == index)[0]
else: # has indexing column
Expand Down Expand Up @@ -150,7 +153,7 @@ def to_table(cosmology, *args, cls=QTable, cosmology_in_meta=True, rename=None):
Parameters
----------
cosmology : |Cosmology| subclass instance
cosmology : `~astropy.cosmology.Cosmology`
The cosmology instance to convert to a table.
*args
Not used. Needed for compatibility with
Expand Down Expand Up @@ -242,7 +245,7 @@ def to_table(cosmology, *args, cls=QTable, cosmology_in_meta=True, rename=None):
tbl = cls(data, meta=meta)

# Renames
renames = {} if rename is None else rename
renames = rename or {}
for name in tbl.colnames:
tbl.rename_column(name, renames.get(name, name))

Expand Down
2 changes: 1 addition & 1 deletion astropy/cosmology/io/tests/test_ecsv.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ def test_readwrite_ecsv_instance(
got = read(fp)
assert got == cosmo

def test_readwrite_ecsv_rename(
def test_readwrite_ecsv_renamed_columns(
self, cosmo_cls, cosmo, read, write, tmp_path, add_cu
):
"""Test rename argument to read/write."""
Expand Down
2 changes: 1 addition & 1 deletion astropy/cosmology/io/tests/test_mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def test_to_mapping_rename_conflict(self, cosmo, to_format):
with pytest.raises(ValueError, match="'renames' values must be disjoint"):
to_format("mapping", rename=to_rename)

def test_from_mapping_rename_disjoint(self, cosmo, to_format, from_format):
def test_from_mapping_rename_conflict(self, cosmo, to_format, from_format):
"""Test ``rename`` in `from_mapping()``."""
m = to_format("mapping")

Expand Down

0 comments on commit d57f2db

Please sign in to comment.