Open Mec-iS opened 1 year ago
cc: @ceteri @tomaarsen
Are you aiming to remove transform
and inverse_transform
in favour of the Pythonic alternatives, or merely also implement the Pythonic alternatives? Obviously the former is a breaking change, while the latter is not.
I'm all for this change, by the way.
yes I would like to remove the current interface. If there are breaking changes, maybe the time for applying them is now, before it stabilises. Anyway we could have both for some time if needed.
Hi @Mec-iS , Change sounds great. Will make usage more intuitive. I intend to take this up.
Good points, all around!
It's fine to have a breaking change, especially now as we're going through major refactoring and will need to bump to a major release update afterwards.
I really like the more pythonic approach!
In terms of breaking changes, we have use cases for this in the tutorial notebooks, which might be reused for unit tests?
Longer-term perspectives (beyond this refactoring):
keyvi
simply to handle the scale of indexing, and that requires the keys (i.e., the node symbols) to be known in advance so that an FST can be built.subgraph
such as having these boundaries based on ML products (e.g., "motifs" or various kinds of embeddings).My main points about these long-term perspectives is to plan on having the indexing process become "pluggable", possibly with some outside framework to handle transform/inverse-transform lookups, and also the construction of some variants of the Subgraph
class may be empirically driven.
I'm submitting a
Current Behaviour:
SubGraph
usestransform
andinverse_transform
to provide Node-to-index and index-to-Node. This is quite non-standard as "trasformations" are usually functions applied to data structures, plus Python has its fluent interface to define this kind of behaviourExpected Behaviour:
The
__getitem__
/__setitem__
protocol should be implemented forSubGraph
so that will be possible to do:to return a node at
i
position. And:to return the index of a given node.
This will allow to use
Subgraph
as an iterator with minor additions.This requires:
inverse_transform
to__getitem__
transform
to__setitem__
Subgraph.index()
For example:
This implies modifying all the existing examples and tests.