glotzerlab/signac

Remove indexing methods from public API

bdice opened this issue · 8 comments

bdice commented

Feature description

In PR #580, I remove a large portion of the indexing.py module. Below are some follow-up tasks to be done after #580 is merged.

  • Investigate making Project.index into a private method. This may eliminate the need for most/all of the contents of indexing.py. (See continued discussion below.)
  • Investigate removing index argument from a number of Project methods. (See continued discussion below.)
  • Review package documentation and remove indexing topics.
  • Review signac-docs to remove indexing topics. see glotzerlab/signac-docs#152
  • Review signac-examples to remove indexing topics.
bdice commented

Discussed in developers meeting. @atravitz and I will work through this checklist and coordinate via Slack.

We agreed that Project.index can be deprecated in 1.x and removed in 2.0, which will eliminate the need for most/all of indexing.py.

Next steps (all of these can be done in parallel):

  • Wait for #580 to be reviewed/merged.
  • In 1.x / master:
    • Deprecate Project.index in 1.x.
    • Add deprecation warnings for all Project methods with non-empty index arguments in 1.x.
  • In 2.0 / next:
    • Make Project.index a private method, remove all unused arguments (only include_job_document should remain, based on current usage).
    • Remove index arguments from all Project methods.
bdice commented

Next steps:

  • Check whether the Project's _sp_index() method and _index_cache attribute are redundant with _sp_cache attribute and _index method. There may also be some overlap with the _build_index or _update_in_memory_cache methods.
  • Do we need to support custom Job subclasses that do not adhere to the same workspace layout? This looks like it's incompletely supported in the Project class and should probably be removed.
    wd = self.workspace() if self.Job is Job else None

The indexing example in signac-examples is already deprecated. Should I remove it now, or wait for the 2.0 release?

bdice commented

@atravitz No need to wait, we can go ahead and modernize and clean up examples!

stale commented

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

bdice commented

Update since this issue is marked as stale:

Next steps:

  • Check whether the Project's _sp_index() method and _index_cache attribute are redundant with _sp_cache attribute and _index method. There may also be some overlap with the _build_index or _update_in_memory_cache methods.

This is under active development in #652. I'll finalize that PR.

  • Do we need to support custom Job subclasses that do not adhere to the same workspace layout? This looks like it's incompletely supported in the Project class and should probably be removed.
    wd = self.workspace() if self.Job is Job else None

I think this still needs to be done. It should be a small PR.

vyasr commented

@bdice is that last task still to-do? Can we close this issue once that's been addressed?

bdice commented

Yes, that is the last task. I opened #693 to resolve this.