refactor: Switch Examples digitization to DirectedProtoAxis#5421
refactor: Switch Examples digitization to DirectedProtoAxis#5421andiwand wants to merge 5 commits into
DirectedProtoAxis#5421Conversation
github-actions
Bot
added
Component - Core
benjaminhuth
left a comment
benjaminhuth
left a comment
There was a problem hiding this comment.
Looks good to me overall! As discussed, a deprecation of ProtoAxis and IAxis::createXXX seems desirable to me...
| } | ||
| j["indices"] = indices; | ||
| j["segmentation"] = nlohmann::json(gdc.segmentation); | ||
| Acts::BinUtility segmentation; |
There was a problem hiding this comment.
just as a question: How big is the overlap here actually? Can we implement a json serialization from vector<axis> that is json compatible with the binutility? so that one could in practice do binutility <-> json <-> vector?
That would make moving forward easier maybe?
There was a problem hiding this comment.
Actually, what I would suggest is:
- when reading old files (for a bit), we should be doing binUnitlity -> json (old) -> vector (new), i.e. support reading old files
- I would not spend time in writing old format, so only have it one way.
There was a problem hiding this comment.
I don't think we version these files and I fear there is no good way to detect old way new way other than trying to parse it and falling back or detecting it by inspecting what properties are present.
in any case I don't think we want to have multiple formats in flight so the easiest way forward IMO is to just stick with the old format as long as we can. out of convenience I did this right now through reading/writing BinUtilities and then converting. but we would also switch over to a different type but still reading from the existing json format
asalzburger
left a comment
asalzburger
left a comment
There was a problem hiding this comment.
For me almost good to go, we should just clarify what we do with backward compatibility,
| } | ||
| j["indices"] = indices; | ||
| j["segmentation"] = nlohmann::json(gdc.segmentation); | ||
| Acts::BinUtility segmentation; |
There was a problem hiding this comment.
Actually, what I would suggest is:
- when reading old files (for a bit), we should be doing binUnitlity -> json (old) -> vector (new), i.e. support reading old files
- I would not spend time in writing old format, so only have it one way.
|
I marks this ready for review, I think it's practically there, and I guess we should make a move. |
…directedprotoaxis-for-digi
|
…directedprotoaxis-for-digi
3 participants
Moves our Examples digitization from
BinUtilitytoDirectedProtoAxis. This gets us one step further to get rid ofBinUtility--- END COMMIT MESSAGE ---
blocked by
MultiAxisfromGrid#5531IAxisgainsAxisDirection#5540grid_helper.hpptoMultiAxisHelper.hpp#5541