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

Change Deme.id to Deme.name #59

Closed
jeromekelleher opened this issue Mar 19, 2021 · 8 comments · Fixed by #61
Closed

Change Deme.id to Deme.name #59

jeromekelleher opened this issue Mar 19, 2021 · 8 comments · Fixed by #61
Assignees

Comments

@jeromekelleher
Copy link
Member

I think we should change the current id attribute on Deme to name. There's two main reasons for this:

  1. Using id is intrusive for downstream implementations as they will already have some concept of a population/deme ID, and it's unlikely to match up with what we're requiring here. For example, msprime/tskit have zero-indexed integers as population IDs, and SLiM has one-indexed integers as population IDs. Neither can map the concept of "Deme id" directly into their data models, requiring icky workarounds where we add a "name" which corresponds to the Demes ID. This is both messy and will confuse users.
  2. It's not as descriptive. "ID" has connotations of some unique, opaque identifier for a thing. We don't want people to give demes IDs, we want people to name then, in meaningful, memorable ways. Calling it a name gets this idea across much better.

Thoughts/objections?

@grahamgower
Copy link
Member

Unfortunately, I think you're right. name would fit in better in a lot of ways.

@grahamgower
Copy link
Member

This seemingly simple change will likely need lots of changes for related variable names and docs.

@jeromekelleher
Copy link
Member Author

Yes, sorry. This will only get worse over time though - who wants to rip off the bandaid? I'll do the spec repo if you do demes-python?

@grahamgower
Copy link
Member

Yes, sorry. This will only get worse over time though - who wants to rip off the bandaid? I'll do the spec repo if you do demes-python?

Deal!

@jeromekelleher
Copy link
Member Author

OK - @molpopgen, @apragsdale any objections?

@molpopgen
Copy link
Contributor

Makes sense to me.

@apragsdale
Copy link
Member

apragsdale commented Mar 19, 2021 via email

@jeromekelleher
Copy link
Member Author

OK, I'll sort this out here when I get a chance.

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

Successfully merging a pull request may close this issue.

4 participants