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.

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

Add tests of DataIdContainer and improve exception handling #54

Merged
merged 2 commits into from Jul 19, 2018

Conversation

parejkoj
Copy link
Contributor

Improving test coverage through improved testing!

Copy link
Contributor

@r-owen r-owen left a comment

Choose a reason for hiding this comment

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

Looks good. One requested addition or change.

raise KeyError("Cannot get keys for datasetType %s at level %s" % (self.datasetType, self.level))
except KeyError as e:
msg = "Cannot get keys for datasetType %s at level %s: %s" % (self.datasetType, self.level)
raise KeyError(msg) from e
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice improvement

# This product includes software developed by the LSST Project
# (https://www.lsst.org).
# See the COPYRIGHT file at the top-level directory of this distribution
# for details of code ownership.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the COPYRIGHT file (or change this not to reference it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@timj came up with a list of things to put in that file. I don't think that process will scale for any other packages that we add new files to...

@parejkoj parejkoj merged commit 5ecaacd into master Jul 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants