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

fix Dict bug #86

Merged
merged 2 commits into from
Feb 27, 2021
Merged

fix Dict bug #86

merged 2 commits into from
Feb 27, 2021

Conversation

CarloLucibello
Copy link
Collaborator

@CarloLucibello CarloLucibello commented Feb 27, 2021

fix #85 introduced in #29 where a Dict{Symbol,T} is reloaded as a BSONDict .

This PR basically reverts #29 and implements #73.

Notice that we still have the bug that Dict{Symbol,T} ir reloaded Dict{Symbol,Any}. That is a long-standing problem that should be fixed elsewhere (I'll file an issue).

@CarloLucibello
Copy link
Collaborator Author

@DhairyaLGandhi

@DhairyaLGandhi
Copy link
Collaborator

The manifest looks weird. I am going to revert the offending pr and merge the other one

@DhairyaLGandhi
Copy link
Collaborator

DhairyaLGandhi commented Feb 27, 2021

Can you remove the bson dump, otherwise looks fine.

@CarloLucibello
Copy link
Collaborator Author

the other one doesn't have tests strict enough, it wouldn't catch future similar bugs

@DhairyaLGandhi
Copy link
Collaborator

Yes we can merge this, instead, but lets not include the bson dump

@DhairyaLGandhi
Copy link
Collaborator

seems like you've removed the bson entirely now?

@CarloLucibello
Copy link
Collaborator Author

CarloLucibello commented Feb 27, 2021

that bson is not part of the tests, is something that probably was inadvertedly uploaded. It is very easy to produce and push it by mistake, e.g. running the examples in the readme, so I added it to .gitignore

@DhairyaLGandhi
Copy link
Collaborator

Sure. Lets have this in and see where we get

@DhairyaLGandhi DhairyaLGandhi merged commit efe5c53 into JuliaIO:master Feb 27, 2021
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