Make workspace a settable property. Define ProjectWorkspace for backwards compatibility

[vyasr]

Description

Makes the workspace of a project modifiable by a property API.

Motivation and Context

Resolves #81. The config is already accessible by property, it's just the workspace that isn't yet.

I do have one question about this feature, and also the config property of the project. Currently any change to the config, including changing the workspace, modifies the in-memory project object, but that change will not persist beyond the current session since the underlying config file isn't updated. Considering that the command line interface for config modification updates the underlying file, and also the way that other things like statepoint/job document modification always persist to the underlying file, is that the expected behavior?

Types of Changes

• Documentation update
• Bug fix
• New feature
• Breaking change¹

¹The change breaks (or has the potential to break) existing functionality.

Checklist:

• I am familiar with the Contributing Guidelines.
• I agree with the terms of the Contributor Agreement.
• My name is on the list of contributors.
• My code follows the code style guideline of this project.
• The changes introduced by this pull request are covered by existing or newly introduced tests.

If necessary:

• I have updated the API documentation as part of the package doc-strings.
• I have created a separate pull request to update the framework documentation on signac-docs and linked it here.
• I have updated the changelog.

[codecov]

Codecov Report

Merging #244 into master will increase coverage by 0.01%.
The diff coverage is 91.3%.

@@            Coverage Diff            @@
##           master    #244      +/-   ##
=========================================
+ Coverage   64.89%   64.9%   +0.01%     
=========================================
  Files          39      39              
  Lines        5569    5577       +8     
=========================================
+ Hits         3614    3620       +6     
- Misses       1955    1957       +2

Impacted Files	Coverage Δ
signac/contrib/project.py	90.62% <91.3%> (-0.19%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85a2db6...16e5693. Read the comment docs.

[vyasr]

I literally just saw that this was mentioned as possibly undesirable in #239. If that is the case, I can consider alternatives for enabling this kind of feature. I do find it useful to be able to modify the workspace, but maybe there's a compromise. This doesn't immediately conflict with the suggestion there since that is a proposed future optimization, but it would preclude that optimization without making an API-breaking change so we can hold off with this PR until that's decided.

[bdice]

I think it would be totally fine to require configuration settings (and workspaces) to be changed via the command line interface, and for configurations to be immutable once a Project is instantiated in Python. Allowing Project configs to change in memory is a great way to introduce bugs, in my opinion. Things like caches can't be used safely without a static config. If you need another workspace in Python, just create a config object and create a new Project instance from that config.

(So while I appreciate your work, I would vote to reject this PR / mutable configs in general.)

[csadorf]

Alternatively you just make the cache workspace path dependent. That would be very easy to realize. It probably would actually be critical, since we already allow an in-memory change of the configuration.

[vyasr]

@csadorf I'm not sure I follow your suggestion.

[bdice]

@csadorf Yes, incorrect caching behavior is probably already possible. I think it makes more sense to make things immutable than to enable multi-workspace caching. I benchmarked this for #239 and saw that signac spends around ~13% of its time just calling project.workspace(), which is silly for a value that is constant in 99.9% of real world use cases. Immutable configs would solve this.

[csadorf]

@csadorf Yes, incorrect caching behavior is probably already possible. I think it makes more sense to make things immutable than to enable multi-workspace caching. I benchmarked this for #239 and saw that signac spends around ~13% of its time just calling project.workspace(), which is silly for a value that is constant in 99.9% of real world use cases. Immutable configs would solve this.

Agreed.

[vyasr]

So what's the expected path forward? Deprecate the ability to update the project config, and remove in 2.0? And otherwise close #81 as wontfix?

[csadorf]

So what's the expected path forward? Deprecate the ability to update the project config, and remove in 2.0? And otherwise close #81 as wontfix?

I'd think so.

[vyasr]

I'd implement that by subclassing the Config class (and making e.g. an ImmutableConfig) that for now throws a DeprecationWarning in __setitem__ that will become an error in 2.0. Sound sufficient?

[vyasr]

This PR will be closed when #246 is merged.

[vyasr]

Closing, replaced by #246.