alarm-redist / redist

Simulation methods for legislative redistricting.
https://alarm-redist.github.io/redist/
GNU General Public License v2.0
66 stars 23 forks source link

Split Redistricting Metrics into `redistmetrics` #91

Closed christopherkenny closed 2 years ago

christopherkenny commented 3 years ago

Package build time is sometimes greater than the 10 minutes allowed by CRAN (and has been at the barrier since just before 3.0.0 release). To continue expanding on simulation methodologies and vignettes, we need to get down the compile time, which is mostly due to the large amount of C++ code in the package.

Proposed Solution:

Needs agreement from @CoryMcCartan, @bfifield, and @kosukeimai as copyright holders (who will retain that as authors of redistmetrics).

kosukeimai commented 3 years ago

What is rict? Another option is to create a separate package for vignettes?

kosukeimai commented 3 years ago

Oh I see it's a new package. There is another package of the same name though it's not on CRAN yet: https://github.com/aquaMetrics/rict

bfifield commented 3 years ago

@christopherkenny - I'm happy to sign off on this. I also think this will be useful for encouraging wider use of the many metrics you've built into redist, including for non-simulation analyses.

CoryMcCartan commented 3 years ago

Yes, will also obviously agree to this.

@kosukeimai basically the motivation is that (1) too much C++ is causing CRAN problems, and (2) the package functionality is getting quite large anyway, with a large number of functions that don't even need to be used with simulated ensembles. Breaking out these functions into another package (which will be imported by redist) will solve both, while not changing anything for current end-users.

kosukeimai commented 3 years ago

That makes sense to me but we should think about the name of the new package. Is there a name that relates to redist?

kosukeimai commented 3 years ago

What about something like redist-metrics as the new package name?

bfifield commented 3 years ago

I think that was the idea from @christopherkenny! redist + rict = redistrict. But I'm open to other name ideas.

christopherkenny commented 3 years ago

Yeah, that was the goal of the name, redist + rict = redistrict.

Package names can only contain letters, numbers, and ".", but use of "." is strongly discouraged as it gets confusing for naming S3 generics.

kosukeimai commented 3 years ago

I think it's hard to connect the two. Maybe, something like redistmetric is better. I would like to keep redist in it.

christopherkenny commented 3 years ago

The difficulty with keeping the name in it is that can have the same effect as we have now, where most people are unaware that there are functions that can work on non-simulated plans.

With the URL, it should be clear that they are related, ie: https://github.com/alarm-redist/rict.

kosukeimai commented 3 years ago

The name redist does not imply simulation methods in itself and so I don't think that will be an issue. No?

christopherkenny commented 3 years ago

I believe Ben and I have both gotten a few messages now from people who were unaware that it could provide metrics outside of a simulation context due to the title of redist (Simulation Methods for Legislative Redistricting).

kosukeimai commented 3 years ago

What I'm saying is that the word redist itself does not imply simulations. So, I don't see a reason why someone who sees redistmetrics would assume it's all about simulations. I would like to keep redist as our brand name if possible.

christopherkenny commented 3 years ago

We can do redistmetrics if you think retaining redist in the name is very important here.

kosukeimai commented 3 years ago

Yes, I think that would be great. It also signals that the package is about redistricting. You can have an informative subtitle which should tell users the metrics are not just about simulations.