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

Store filenames #4

Merged
merged 4 commits into from Feb 20, 2015
Merged

Store filenames #4

merged 4 commits into from Feb 20, 2015

Conversation

maxhutch
Copy link

Filenames are stored along with keys, making old chests more portable. _keys, which was a set, is replaced with a {key -> filename} dictionary. The dictionary is dumped/loaded as a list of tuples for json compatibility, but used as a dict internally.

@@ -154,7 +154,7 @@ def __getitem__(self, key):
if key in self.inmem:
value = self.inmem[key]
else:
if key not in self._keys:
if key not in self._keys.keys():
Copy link
Member

Choose a reason for hiding this comment

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

The old version is probably best in my_dict actually checks for membership in the keys. Adding the .keys() method creates an explicit list.

@mrocklin
Copy link
Member

I'm not entirely sure what's happening with the file not found issue (though I also haven't looked very deeply into it). During garbage collection (__del__ call) we drop the chest. Some file seems to be missing. Perhaps we're trying to delete a file that isn't written or deleting a file twice?

@mrocklin
Copy link
Member

I suspect that the coverage error in Python3 might be my fault. I think it's in master.

@mrocklin
Copy link
Member

I'll take a look at that error in the next day or so. Happy to merge this with this error.

When the dict is treated like a set, it uses the set of keys.  Also,
list() knows how to handle dictionary view objects without an
intermediate cast.
@mrocklin
Copy link
Member

Happy to merge. Was there any behavior you wanted to test to make sure that there isn't a regression in the future? Currently there is nothing stopping someone else from coming along and unknowingly changing things back.

@maxhutch
Copy link
Author

Good call. I wonder why that with self.lock() line keeps failing coveralls...

@mrocklin
Copy link
Member

It's never being triggered by any of the tests. I'm investigating that this afternoon.

mrocklin added a commit that referenced this pull request Feb 20, 2015
@mrocklin mrocklin merged commit a155288 into blaze:master Feb 20, 2015
@maxhutch
Copy link
Author

Its non-deterministic. On my local machine:

maxhutch@edoras:~/src/chest$ ~/anaconda3/bin/coverage run --source=chest --omit=chest/tests/test* ~/anaconda3/bin/nosetests --with-doctest 
...
Ran 33 tests in 0.405s

OK
maxhutch@edoras:~/src/chest$ ~/anaconda3/bin/coverage report
Name             Stmts   Miss  Cover
------------------------------------
chest/__init__       2      0   100%
chest/core         168      0   100%
chest/utils          6      0   100%
------------------------------------
TOTAL              176      0   100%
maxhutch@edoras:~/src/chest$ ~/anaconda3/bin/coverage run --source=chest --omit=chest/tests/test* ~/anaconda3/bin/nosetests --with-doctest 
...
Ran 33 tests in 0.408s

OK
maxhutch@edoras:~/src/chest$ ~/anaconda3/bin/coverage report
Name             Stmts   Miss  Cover
------------------------------------
chest/__init__       2      0   100%
chest/core         168      1    99%
chest/utils          6      0   100%
------------------------------------
TOTAL              176      1    99%

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