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

Make an option to include teiHeader in getTextualNode() #148

Open
PonteIneptique opened this issue Jan 5, 2018 · 2 comments
Open

Make an option to include teiHeader in getTextualNode() #148

PonteIneptique opened this issue Jan 5, 2018 · 2 comments
Assignees
Milestone

Comments

@PonteIneptique
Copy link
Member

We need to make it possible to export the teiHeader at .getTextualNode(). We'll also need an option somewhere in the Retriever but first we need that at the getTextualNode() level.

I was wondering if @brosner or @jtauber had thoughts about this question and how they'd see the function get expanded from where it is now.

My feeling is include_header is too TEI specific, but the metadata argument is already taken in the resolver declaration (and though would have a different meaning in between classes type which would be bad design I think). Any idea ?

Current implementation

Text Class

For reference : https://github.com/Capitains/MyCapytain/blob/master/MyCapytain/resources/texts/local/capitains/cts.py#L46-L100

Function declaration :

def getTextualNode(self, subreference=None, simple=False):

Resolver

https://github.com/Capitains/MyCapytain/blob/master/MyCapytain/resolvers/prototypes.py#L28-L42

    def getTextualNode(self, textId, subreference=None, prevnext=False, metadata=False):
        """ Retrieve a text node from the API
        :param textId: CtsTextMetadata Identifier
        :type textId: str
        :param subreference: CapitainsCtsPassage Reference
        :type subreference: str
        :param prevnext: Retrieve graph representing previous and next passage
        :type prevnext: boolean
        :param metadata: Retrieve metadata about the passage and the text
        :type metadata: boolean
        :return: CapitainsCtsPassage
        :rtype: CapitainsCtsPassage
        """
        raise NotImplementedError()
@brosner
Copy link
Contributor

brosner commented Jan 19, 2018

I am not sure I understand how the TEI header relates to the textual node offered by MyCapytain. While I get that it exists in the TEI and the textual node is coming from it, I think it needs its own dedicated method.

For example, including the header within any other value for the other parameters seems confusing to me. Surely a caller would only need the header once and not optionally for each getTextualNode invocation. Wrong level of abstraction IMO.

@PonteIneptique
Copy link
Member Author

The teiHeader and other nodes sometimes (like Front and Back) might hold information necessary to understand and read information in the textual node (for example glossaries). This is extremely common in critical edition (which Perseus does not have at the moment).

I like the idea of it having it's own function (ie Passage.add(SomeXpath)) but I need to see the implication of such a choice outside of the resolver (for local files without resolver, I do not remember if texts are tied with their passage)

Note : This is coming as a requirement of DTS as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants