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

reorg for new users exploring data model #172

Merged
merged 6 commits into from Jun 6, 2023
Merged

reorg for new users exploring data model #172

merged 6 commits into from Jun 6, 2023

Conversation

sbailey
Copy link
Contributor

@sbailey sbailey commented Jun 5, 2023

This PR is a work in progress to reorganize the data model presentation to be more oriented towards new users exploring the datamodel and directory tree, rather than expert developers:

  • Top level landing page splits most commonly used directories (e.g. spectro/redux/ and target/) from historical/expert directories (like protodesi/)
  • Non-directory things (like mask bits) get their own bullet list section instead of being mixed in with directories.
  • Landing page and descriptions emphasize the default path relative to $DESI_ROOT rather than the expert-level environment variables (e.g. $DESI_ROOT/spectro/redux over $DESI_SPECTRO_REDUX).
  • The environment variables that point to subdirs under $DESI_ROOT are moved to a separate page rather than being the primary starting point, since most new users won't know what $DESI_SPECTRO_REDUX is how how it relates to the EDR root directory.

So far I've just updated the landing page and the $DESI_ROOT/spectro/data tree (while admitting that isn't the most commonly used dir by end-users). I'm posting this PR as a WIP now so that others (especially @weaverba137) can look and comment if needed before I spend too much more time adding similar changes to the DESI_SPECTRO_REDUX tree.

Other than the addition of envvars.rst, this PR (and future updates to it) only modifies index.rst files, not the core directory structure nor the data model files themselves.

@sbailey sbailey requested a review from weaverba137 June 5, 2023 23:18
@coveralls
Copy link

coveralls commented Jun 5, 2023

Coverage Status

coverage: 100.0%. remained the same when pulling 4cda055 on exploring into 521f87e on main.

Copy link
Member

@weaverba137 weaverba137 left a comment

Choose a reason for hiding this comment

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

This looks good. I had only some minor suggestions that should be easy to implement.

doc/DESI_SPECTRO_DATA/index.rst Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
doc/index.rst Show resolved Hide resolved
doc/index.rst Show resolved Hide resolved
@sbailey sbailey marked this pull request as ready for review June 6, 2023 03:49
@sbailey
Copy link
Contributor Author

sbailey commented Jun 6, 2023

@weaverba137 thanks for the pre-review; this is now ready for final review. The main updates are to the landing page and the pages directly linked from the landing page. In the interest of time I'm cutting myself off descending to deeper levels of linking, though that could use some more cleanup in the future.

Copy link
Member

@weaverba137 weaverba137 left a comment

Choose a reason for hiding this comment

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

Looks good to merge now.

@sbailey
Copy link
Contributor Author

sbailey commented Jun 6, 2023

Thanks. I also addressed the csv quoting issues in column_descriptions.csv to fix #174 so that github displays it nicely, but I didn't go all the way to #175 to render it into rst automatically.

@sbailey sbailey merged commit cc70799 into main Jun 6, 2023
22 checks passed
@sbailey sbailey deleted the exploring branch June 6, 2023 22:56
@sbailey sbailey self-assigned this Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants