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

Make Keys, Values, and ItemsView subscriptable #25

Merged
merged 4 commits into from
Mar 30, 2017

Conversation

percurnicus
Copy link
Member

Fixes #24

KeysView, ValuesView, and ValuesView now have a __getitem__ method so they can be subscriptable in a similar way that keys(), values(), and items() are subscriptable for normal dictionaries. I also adjusted their __repr__ methods to show the correct information in tandem with their dictionary method counterpart.

Example:

In [1]: import pvl

In [2]: module = pvl.PVLModule([
   ...:         ('a', 1),
   ...:         ('b', 2),
   ...:         ('a', 3),
   ...:     ])

In [3]: module.items()[0]
Out[3]: ('a', 1)

In [4]: module = pvl.load('tests/data/pattern.cub')

In [5]: module.keys()
Out[5]: KeysView(['IsisCube', 'Label'])

In [6]: module.values()[1]
Out[6]: 
PVLObject([
  ('Bytes', 65536)
])

@percurnicus
Copy link
Member Author

All tests pass is ready for review @godber @wtolson

@godber
Copy link
Member

godber commented Mar 30, 2017

@pbvarga1 this looks good. I think there's one suggestion that I would have for extending the tests. What if you had a bug in your implementation that always returned the first item instead of returning the correct item. Your existing tests would not catch that bug since you only ever test the first value. I will accept the PR without that change, however.

Also, we should do some talking with former stake holders about what to do with this project. I don't want you guys to have to rely on us who are increasingly less involved.

@godber godber self-requested a review March 30, 2017 13:35
@godber
Copy link
Member

godber commented Mar 30, 2017

Tests can be improved after the fact if desired. Accepting.

@godber godber merged commit 27b4128 into planetarypy:master Mar 30, 2017
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