-
Notifications
You must be signed in to change notification settings - Fork 2
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
Preparation for v0.19 #918
Conversation
Codecov Report
@@ Coverage Diff @@
## master #918 +/- ##
==========================================
- Coverage 76.06% 73.44% -2.63%
==========================================
Files 38 27 -11
Lines 3134 2154 -980
==========================================
- Hits 2384 1582 -802
+ Misses 750 572 -178
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
All these functions should stay in the abstract interface, and if new drivers are not immediately able to implement them then that is just a limitation of that driver. Other DFG drivers like GraphDFG already implement them so nothing has to change there. Also functions that exist in AbstactDFG only made it there because they were used somewhere. We have very little fat in DFG so to speak. |
thanks for prepping the PR! |
We should just put Neo4jDFG under a whole new repo and distribute with GPL license? See Stefan K's issue: |
…aphs.jl into feature/prep_v019
…aphs.jl into feature/prep_v019
…aphs.jl into feature/prep_v019
src/services/Serialization.jl
Outdated
smallData = JSON2.read(packedProps["smallData"], Dict{Symbol, SmallDataTypes}) | ||
|
||
# Smalldata refactor to metadata | ||
if (haskey(packedProps, "metadata")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to help, on master branch i added additional option in if packedProps is no longer a JSON string inside the top level JSONable object:
DistributedFactorGraphs.jl/src/services/Serialization.jl
Lines 348 to 360 in 685b5f9
smallData = if haskey(packedProps, "smallData") | |
if packedProps["smallData"] isa String | |
JSON2.read(packedProps["smallData"], Dict{Symbol, SmallDataTypes}) | |
elseif packedProps["smallData"] isa Dict | |
Dict{Symbol, SmallDataTypes}( Symbol.(keys(packedProps["smallData"])) .=> values(packedProps["smallData"]) ) | |
# packedProps["smallData"] | |
else | |
@warn "unknown smallData deserialization on $label, type $(typeof(packedProps["smallData"]))" maxlog=10 | |
Dict{Symbol, SmallDataTypes}() | |
end | |
else | |
Dict{Symbol, SmallDataTypes}() | |
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me
Remove LightDFG and deprecate with error
…tedFactorGraphs.jl into feature/prep_v019
Co-authored-by: Dehann Fourie <dehann@users.noreply.github.com>
This is the precursor to resolving #590 |
-5,700 lines... yep 🧹 🔥 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i had another fix or two to add, and looks good to me. Tests on IIF also pass.
This is in preparation for the v0.19 release as we discussed @dehann @Affie.
Open question(s) (@Affie)
Regarding downstream the DFG driver we are planning on building out, I believe these functions will be difficult to implement:
I suggest we leave this open until we are ready to make v0.19 (no urgency), I just set it up while reviewing the code.