glotzerlab / signac-flow

Workflow management for signac-managed data spaces.
https://signac.io/
BSD 3-Clause "New" or "Revised" License
48 stars 37 forks source link

Better naming of types of aggregates #274

Open vyasr opened 4 years ago

vyasr commented 4 years ago

FlowGroups provide a mechanism for grouping operations. We have proposals for job aggregation, and we also use the term bundle for groups of cluster jobs. We will also need to formalize a concept of aggregation for directives as part of #265. We're already seeing that the similarity in these terms is leading to confusion for developers. We should look into giving these concepts more descriptive names to reduce confusion about their individual roles.

kidrahahjo commented 4 years ago

@vyasr Please assign me this issue, I'll start working on this.

csadorf commented 4 years ago

Generally in favor in working out a comprehensive and coherent glossary.

However, I'd suggest to avoid renaming previous concepts to avoid creating more confusion, especially since groups have just been released.

vyasr commented 4 years ago

I agree, but I think we can do it by just augmenting existing names without losing the existing association, for instance consistently referring to bundles as "submission bundles", groups as "operation groups", and aggregates as "job aggregates" (or whatever terms we pick). Also I think a large part of this can come in the form of documentation more than anything else, I don't think we would need to change class names (although that may become more feasible depending on how much we "privatize" the API).

csadorf commented 4 years ago

Let's work out the glossary first. Hopefully there is no strong need to rename anything as part of the API.

kidrahahjo commented 4 years ago

Since we're converting internally everything into aggregates, I think the methods like _create_run_job_operations should be renamed to maybe _create_run_job_aggregate_operations and similarly the class _JobOperation to _JobAggregateOperation.

I'll be happy to work on this after #336 gets merged.