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

Schema2 #742

Merged
merged 9 commits into from
Apr 19, 2022
Merged

Schema2 #742

merged 9 commits into from
Apr 19, 2022

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Apr 14, 2022

Description

This PR should be merged using a merge commit, not squashed and rebased
This pull request implements the migration from signac schema version 1 to schema version 2. The primary change is that all of signac's internal files are stored in a single common directory .signac in the project root instead of being stored directly in the project root. In particular, the signac config file signac.rc is now instead store in .signac/config.

Motivation and Context

Resolves #197. See the constituent commits to see the other associated issues.

Checklist:

@vyasr vyasr added the enhancement New feature or request label Apr 14, 2022
@vyasr vyasr added this to the v2.0.0 milestone Apr 14, 2022
@vyasr vyasr requested review from a team as code owners April 14, 2022 05:48
@vyasr vyasr self-assigned this Apr 14, 2022
@vyasr vyasr requested review from mikemhenry and klywang and removed request for a team April 14, 2022 05:48
Copy link
Member

@bdice bdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on board with all these changes, since we've gone through separate PR reviews on each of them. On a second reading, I see a number of TODOs that we will want to resolve before releasing 2.0. We will probably want to grep for VERSION_3 and make those changes for 2.0 as we discussed recently. We will also want to remove the compatibility layers for init_project / get_project. Feel free to merge when ready.

@bdice
Copy link
Member

bdice commented Apr 17, 2022

Oh, and we'll need to update our pre-commit hooks. The black pinning issue is showing up here. @vyasr I'll let you rebase as needed, since I've lost track of exactly how the rebase needs to work for schema2. I'm also fine with removing the pre-commit CI requirement, force-merging this PR, re-enabling it, and then rebasing next if it reduces the number of rebases required.

vyasr and others added 8 commits April 19, 2022 00:40
* Implement initial migration to schema version 2.

* Require project-local config to be signac.rc (not .signacrc) and make searches stricter to match.

* Standardize method for getting project config at a root.

* Move config to .signac/config.

* Fix import order.

* Address PR comments.

* Remove some unnecessary code.

* Address final PR coments.
* Change schema versioning to use integer strings.

* Switch from int strings to pure ints.

* Update signac/contrib/migration/__init__.py

Co-authored-by: Bradley Dice <[email protected]>

Co-authored-by: Bradley Dice <[email protected]>
* Remove project id API.

* Remove project name from config as part of migration.

* Fix issues with config CLI and remove project from default cfg.

* Address PR comments.

* Change the str of a project to the str of its root directory.
* Change project constructor to accept a root directory instead of a config file.

* Change Project repr.

* Address easy PR comments.
* Move shell history.

* Move sp_cache file.

* Address PR comments.
* Move discovery to separate functions.

* Address first round of PR comments.

* Address PR comments.

* Apply suggestions.
* Remove workspace configurability.

* Implement workspace_dir migration.

* Apply suggestions from code review

Co-authored-by: Bradley Dice <[email protected]>

* Address remaining PR comments.

* Update tests/test_project.py

* Remove mention of configurability from project workspace docstring

* Address PR comments.

Co-authored-by: Bradley Dice <[email protected]>
Co-authored-by: Corwin Kerr <[email protected]>
@vyasr
Copy link
Contributor Author

vyasr commented Apr 19, 2022

The current master->next rebase is super gnarly and I screwed it up the first couple of times that I tried it right after making this PR. I just got it done correctly, so I can push now and it should resolve these problems.

@vyasr vyasr merged commit 36cdc29 into next Apr 19, 2022
@vyasr vyasr deleted the schema2 branch April 19, 2022 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants