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

Refactor for 2.0 #13

Closed xrotwang closed 4 years ago

xrotwang commented 4 years ago

Since the attributes corresponding to blocks are set via setattr (see https://github.com/SimonGreenhill/python-nexus/blob/a090ecf4c2b3fc45b1099ad8cc9058745c9fd418/nexus/reader.py#L45) they cannot be inferred by IDEs, which makes the library harder to use.

It's also a bit more clunky, if not all attributes are set at all times, and hasattr must be used when if reader.trees should be sufficient (e.g. if the handlers had appropriate __bool__ methods).

SimonGreenhill commented 4 years ago

Ok, so what's the solution here -- set .trees, .data etc in __init__ to None, and override when present? or make these properties that return None if not found?

(This whole library needs a good refactor :/)

xrotwang commented 4 years ago

Ok, I'll take a stab at refactoring :) I guess parsing the [&R|U] syntax should live in this library as well, right?

SimonGreenhill commented 4 years ago

Much appreciated!

As for [&R], yes in here, or we could make the newick library a dependency, and make that able to parse trees in the extended nexus format (incl. name and [&R])? (i.e. so python-nexus handles nexus parsing, newick does the trees)

xrotwang commented 4 years ago

@SimonGreenhill are you ok with re-arranging the source tree, i.e. moving the package to src, moving test up to the project dir, moving examples into test/fixtures? Ah, and dropping py2 compat?

SimonGreenhill commented 4 years ago

yes, yes, and yes. Let's make it a new major release though (2.0?)

xrotwang commented 4 years ago

What about moving the repos to https://github.com/glottobank (or possibly an org that fits even better - glottobank already has python-newick, so that's what may make it suitable)?

SimonGreenhill commented 4 years ago

ok -- glottobank? we also have https://github.com/shh-dlce/, but this looks to be largely outdated things?

xrotwang commented 4 years ago

ah right. yes, that's better. It's not outdated things only. It also has appconfig.

xrotwang commented 4 years ago

I'd also rewrite the cli to just provide one nexus command with subcommands - as for most of our other packages. Or do you think that's too much of a breaking change?

SimonGreenhill commented 4 years ago

Just moved here

SimonGreenhill commented 4 years ago

Yes, redoing the cli interface would be really nice too, it's accumulated a lot of cruft.

I think these can be deleted:

Related to characters/data:

Related to trees:

Perhaps a 'describe' command for:

Perhaps a 'convert' command:

edit: perhaps this https://github.com/shh-dlce/python-nexus/blob/master/nexus/bin/nexuscheck.py can be a 'check' command.

xrotwang commented 4 years ago

One more thing: You might want to "release" the association between your personal user with the repos on Zenodo. Then we could hook it up with the institutional one.

SimonGreenhill commented 4 years ago

ok, will try that -- once I figure out where to look on zenodo.

xrotwang commented 4 years ago

you login with github, and then open the github link from the account drop-down. This will open a list of repos, each with a switch button.

xrotwang commented 4 years ago

ah, looks like you don't have to do that. Apparently the association didn't survive the repos move.

SimonGreenhill commented 4 years ago

yeah it's not listed in the repos list. Must have been unlinked with the move?

xrotwang commented 4 years ago

@SimonGreenhill should we require clldutils?

I'd use clldutils mainly for the cli. So this would pull in some things for no good reason: https://github.com/clld/clldutils/blob/92ab4cab4f9a39e0d6f20f09ef75e0ea4a11d025/setup.py#L15-L21 But then, once they are there, some may come in handy?

SimonGreenhill commented 4 years ago

sure!