Closed thazhemadam closed 3 years ago
Merging #91 (99dd901) into main (6123ed5) will increase coverage by
2.27%
. The diff coverage is65.51%
.:exclamation: Current head 99dd901 differs from pull request most recent head bdd35ec. Consider uploading reports for the commit bdd35ec to get more accurate results
@@ Coverage Diff @@
## main #91 +/- ##
==========================================
+ Coverage 83.01% 85.28% +2.27%
==========================================
Files 11 12 +1
Lines 312 299 -13
==========================================
- Hits 259 255 -4
+ Misses 53 44 -9
Impacted Files | Coverage Δ | |
---|---|---|
src/ChemistryFeaturization.jl | 66.66% <0.00%> (-33.34%) |
:arrow_down: |
src/features/features.jl | 66.66% <0.00%> (-33.34%) |
:arrow_down: |
src/atoms/atomgraph.jl | 72.58% <61.11%> (+10.24%) |
:arrow_up: |
src/featurizedatoms.jl | 83.33% <83.33%> (ø) |
|
src/featurizations/featurizations.jl | 100.00% <100.00%> (ø) |
|
src/featurizations/graphnodefeaturization.jl | 90.69% <100.00%> (-1.31%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 6123ed5...bdd35ec. Read the comment docs.
Presently, the
featurization
inAtomGraph
is present for effective serialization support. However, this is not without its issues (see #85). However, instead of adding a new featurization type, I propose adding a new type that encapsulates the data that would be serialized otherwise, and use this for serialization purposes (and more). This new type would essentially contain anAbstractAtoms
object,AbstractFeaturization
object, and anencoded_features
field. The features stored inencoded_features
can always be decoded using theAbstractFeaturization
associated.Some of the pros that this approach offers -
AbstractAtom
, as they won't need to have thefeaturization
fieldAtomGraph
can be simplified. This would also apply to other types ofAbstractAtoms
types (as they're developed).