Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DM-19873: Resurrect get() and add __getitem__ calling it #50

Merged
merged 4 commits into from
May 28, 2019
Merged

DM-19873: Resurrect get() and add __getitem__ calling it #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
da4a7e0
Resurrect get() and add __getitem__ calling it
timj May 24, 2019
acbc2c7
Add PropertySet.update
timj May 24, 2019
9514417
Some clean ups of doc strings
timj May 28, 2019
a4d812c
Conform the package to 79 character docstrings.
timj May 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 10 additions & 7 deletions python/lsst/daf/base/citizen/citizenContinued.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,16 @@


def setCallbacks(new=None, delete=None, both=False):
"""Set the callback IDs for the Citizen; if both is true, set both new and delete to the same value
"""Set the callback IDs for the Citizen; if both is true, set both new and
delete to the same value.
You probably want to chant the following to gdb:
break defaultNewCallback
break defaultDeleteCallback
* break defaultNewCallback
* break defaultDeleteCallback
You might want to put this in your .gdbinit file.
You might want to put this in your ``.gdbinit`` file.
You can retrieve a citizen's signature from python with obj.repr()
You can retrieve a citizen's signature from python with ``obj.repr()``.
"""

if both:
Expand All @@ -60,8 +61,10 @@ def mortal(memId0=0, nleakPrintMax=20, first=True, showTypes=None):
@param memId0 Only consider blocks allocated after this memId
@param nleakPrintMax Maximum number of leaks to print; <= 0 means unlimited
@param first Print the first nleakPrintMax blocks; if False print the last blocks.
@param showTypes Only print objects matching this regex (if starts with !, print objects that don't match)
@param first Print the first nleakPrintMax blocks; if False print the last
blocks.
@param showTypes Only print objects matching this regex (if starts with !,
print objects that don't match)
If you want finer control than nleakPrintMax/first provide, use
Citizen.census() to get the entire list
Expand Down
201 changes: 153 additions & 48 deletions python/lsst/daf/base/propertyContainer/propertyContainerContinued.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@

import enum
import numbers
import warnings
from collections.abc import Mapping, KeysView

# Ensure that C++ exceptions are properly translated to Python
Expand Down Expand Up @@ -181,11 +180,11 @@ def _propertyContainerGet(container, name, returnStyle):
Raises
------
KeyError
The specified key does not exist in the container.
Raised if the specified key does not exist in the container.
TypeError
The value retrieved is of an unexpected type.
Raised if the value retrieved is of an unexpected type.
ValueError
The value for ``returnStyle`` is not correct.
Raised if the value for ``returnStyle`` is not correct.
"""
if not container.exists(name):
raise KeyError(name + " not found")
Expand Down Expand Up @@ -238,8 +237,8 @@ def _guessIntegerType(container, name, value):
try:
containerType = _propertyContainerElementTypeName(container, name)
except LookupError:
# nothing in the container so choose based on size. Safe option is to
# always use LongLong
# nothing in the container so choose based on size. Safe option is
# to always use LongLong
if value <= maxInt and value >= minInt:
useType = "Int"
else:
Expand All @@ -248,8 +247,8 @@ def _guessIntegerType(container, name, value):
if containerType == "Int":
# Always use an Int even if we know it won't fit. The later
# code will trigger OverflowError if appropriate. Setting the
# type to LongLong here will trigger a TypeError instead so it's
# best to trigger a predictable OverflowError.
# type to LongLong here will trigger a TypeError instead so
# it's best to trigger a predictable OverflowError.
useType = "Int"
elif containerType == "LongLong":
useType = "LongLong"
Expand Down Expand Up @@ -340,27 +339,32 @@ class PropertySet:
None: "Undef",
}

def get(self, name):
"""Return an item as a scalar or array
def get(self, name, default=None):
"""Return an item as a scalar, else default.
Return an array if the item is of numeric or string type and has
more than one value, otherwise return a scalar.
.. deprecated:: 20180-06
`get` is superseded by `getArray` or `getScalar`
Identical to `getScalar` except that a default value is returned
if the requested key is not present. If an array item is requested
the final value in the array will be returned.
Parameters
----------
name : ``str``
Name of item
default : `object`, optional
Default value to use if the named item is not present.
Raises
------
KeyError
If the item does not exist.
Returns
-------
value : any type supported by container
Single value of any type supported by the container, else the
timj marked this conversation as resolved.
Show resolved Hide resolved
default value if the requested item is not present in the
container. For array items the most recently added value is
returned.
"""
warnings.warn("Use getArray or getScalar instead", DeprecationWarning, stacklevel=2)
return _propertyContainerGet(self, name, returnStyle=ReturnStyle.AUTO)
try:
return _propertyContainerGet(self, name, returnStyle=ReturnStyle.SCALAR)
except KeyError:
return default

def getArray(self, name):
"""Return an item as an array if the item is numeric or string
Expand All @@ -373,27 +377,38 @@ def getArray(self, name):
name : `str`
Name of item
Returns
-------
values : `list` of any type supported by container
The contents of the item, guaranteed to be returned as a `list.`
Raises
------
KeyError
If the item does not exist.
Raised if the item does not exist.
"""
return _propertyContainerGet(self, name, returnStyle=ReturnStyle.ARRAY)

def getScalar(self, name):
"""Return an item as a scalar
If the item has more than one value then the last value is returned
If the item has more than one value then the last value is returned.
Parameters
----------
name : `str`
Name of item
Returns
-------
value : scalar item
Value stored in the item. If the item refers to an array the
most recently added value is returned.
Raises
------
KeyError
If the item does not exist.
Raised if the item does not exist.
"""
return _propertyContainerGet(self, name, returnStyle=ReturnStyle.SCALAR)

Expand Down Expand Up @@ -436,11 +451,33 @@ def add(self, name, value):
Raises
------
lsst::pex::exceptions::TypeError
If the type of `value` is incompatible with the existing value
of the item.
Raised if the type of `value` is incompatible with the existing
value of the item.
"""
return _propertyContainerAdd(self, name, value, self._typeMenu)

def update(self, addition):
"""Update the current container with the supplied additions.
Parameters
----------
addition : `collections.abc.Mapping` or `PropertySet`
The content to merge into the current container.
Notes
-----
If the supplied parameter is a `PropertySet` then the
`PropertySet.combine` method will be used. If the supplied parameter
is a `collections.abc.Mapping` each item will be copied out of the
mapping and value assigned using `PropertySet.set`, overwriting
any previous values.
"""
if isinstance(addition, PropertySet):
self.combine(addition)
else:
for k, v in addition.items():
self[k] = v
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be much happier if you omitted this unless you really need it. I think it goes a bit too far in treating PropertySet like a dict.

What does this do if the key already exists? Does this add the new value or replace the existing values? If you must have this then please be much more explicit about its behavior in the documentation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add more explanation. I added this because I had a direct need for it.


def toDict(self):
"""Returns a (possibly nested) dictionary with all properties.
Expand Down Expand Up @@ -489,10 +526,33 @@ def __deepcopy__(self, memo):
return result

def __contains__(self, name):
# Do not use exists() because that includes "."-delimited names
"""Determines if the name is found at the top level hierarchy
of the container.
Notes
------
Does not use `PropertySet.exists()`` because that includes support
for "."-delimited names. This method is consistent with the
items returned from ``__iter__``.
"""
return name in self.names(topLevelOnly=True)

def __setitem__(self, name, value):
"""Assigns the supplied value to the container.
Parameters
----------
name : `str`
Name of item to update.
value : Value to assign
Can be any value supported by the container's ``set()``
method. `~collections.abc.Mapping` are converted to
`PropertySet` before assignment.
Notes
-----
Uses `PropertySet.set`, overwriting any previous value.
"""
if isinstance(value, Mapping):
# Create a property set instead
ps = PropertySet()
Expand All @@ -501,6 +561,16 @@ def __setitem__(self, name, value):
value = ps
self.set(name, value)

def __getitem__(self, name):
"""Returns a scalar item from the container.
Notes
-----
Uses `PropertySet.getScalar` to guarantee that a single value
will be returned.
"""
return self.getScalar(name)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please document what this does in a doc string, if you can figure out a sensible place to put it. I suspect for it to be easily understood it should be in the doc string for the class, but maybe a doc string here would work if sphinx is smart enough.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

daf_base still uses doxygen and I think the best place to document the dict-like behavior of PropertySet would be in the index.rst file once it's ported to sphinx. There doesn't seem to be a doxygen root page that I can augment.

def __delitem__(self, name):
if name in self:
self.remove(name)
Expand Down Expand Up @@ -543,58 +613,76 @@ class PropertyList:
}

COMMENTSUFFIX = "#COMMENT"
"""Special suffix used to indicate that a named item being assigned
using dict syntax is referring to a comment, not value."""

def get(self, name):
"""Return an item as a scalar or array
Return an array if the item has more than one value,
otherwise return a scalar.
def get(self, name, default=None):
"""Return an item as a scalar, else default.
.. deprecated:: 20180-06
`get` is superseded by `getArray` or `getScalar`
Identical to `getScalar` except that a default value is returned
if the requested key is not present. If an array item is requested
the final value in the array will be returned.
Parameters
----------
name : `str`
name : ``str``
Name of item
default : `object`, optional
Default value to use if the named item is not present.
Raises
------
KeyError
If the item does not exist.
Returns
-------
value : any type supported by container
Single value of any type supported by the container, else the
default value if the requested item is not present in the
container. For array items the most recently added value is
returned.
"""
warnings.warn("Use getArray or getScalar instead", DeprecationWarning, stacklevel=2)
return _propertyContainerGet(self, name, returnStyle=ReturnStyle.AUTO)
try:
return _propertyContainerGet(self, name, returnStyle=ReturnStyle.SCALAR)
except KeyError:
return default

def getArray(self, name):
"""Return an item as an array
"""Return an item as a list.
Parameters
----------
name : `str`
Name of item
Returns
-------
values : `list` of values
The contents of the item, guaranteed to be returned as a `list.`
Raises
------
KeyError
If the item does not exist.
Raised if the item does not exist.
"""
return _propertyContainerGet(self, name, returnStyle=ReturnStyle.ARRAY)

def getScalar(self, name):
"""Return an item as a scalar
If the item has more than one value then the last value is returned
If the item has more than one value then the last value is returned.
Parameters
----------
name : `str`
Name of item
Name of item.
Returns
-------
value : scalar item
Value stored in the item. If the item refers to an array the
most recently added value is returned.
Raises
------
KeyError
If the item does not exist.
Raised if the item does not exist.
"""
return _propertyContainerGet(self, name, returnStyle=ReturnStyle.SCALAR)

Expand Down Expand Up @@ -641,8 +729,8 @@ def add(self, name, value, comment=None):
Raises
------
lsst::pex::exceptions::TypeError
If the type of `value` is incompatible with the existing value
of the item.
Raise if the type of ``value`` is incompatible with the existing
value of the item.
"""
args = []
if comment is not None:
Expand Down Expand Up @@ -742,6 +830,23 @@ def __iter__(self):
yield n

def __setitem__(self, name, value):
"""Assigns the supplied value to the container.
Parameters
----------
name : `str`
Name of item to update. If the name ends with
`PropertyList.COMMENTSUFFIX`, the comment is updated rather
than the value.
value : Value to assign
Can be any value supported by the container's ``set()``
method. `~collections.abc.Mapping` are converted to
`PropertySet` before assignment.
Notes
-----
Uses `PropertySet.set`, overwriting any previous value.
"""
if name.endswith(self.COMMENTSUFFIX):
name = name[:-len(self.COMMENTSUFFIX)]
self.setComment(name, value)
Expand Down