dlce-eva / python-nexus

python-nexus - Generic nexus (.nex, .trees) reader/writer for python
BSD 2-Clause "Simplified" License
11 stars 3 forks source link

Breaking changes #14

Closed xrotwang closed 4 years ago

xrotwang commented 4 years ago

I'm somewhat reluctant to break backwards compatibility for stylistic reasons, but NexusReader.read_string and NexusReader.read_file are just a bit weird, in particular the former, even returning self.

I'd say the "real" API should look like this:

class NexusReader:
    def __init__(**blocks):
        self.blocks = {block: self.handlers.get(block, GenericHandler)(lines) for block, lines in blocks.items()}

    @staticmethod
    def iter_blocks(handle):
        ...
        yield block, lines

    @classmethod
    def from_file(cls, filename):
        ...
        return cls(**dict(NexusReader.iter_blocks(handle)))

    @classmethod
    def from_string(cls, strin):
        return cls(...)

One could add read_file back in, and deprecate it. But would you think that's worth it? I.e. how much code depends on the NexusReader API?

xrotwang commented 4 years ago

Ok, bolting on the factory method API while keeping the old one isn't too hard. But I'd still deprecrate read_file and read_string, ok?

SimonGreenhill commented 4 years ago

Yes, deprecate. I think most accesses I know of are via NexusReader(filename), and the others (especially read_string) were mainly used to make tests better.