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

docs: 📝 add examples to user facing functions #982

Merged
merged 13 commits into from
Jan 24, 2025

Conversation

signekb
Copy link
Member

@signekb signekb commented Jan 21, 2025

Description

This PR adds examples in the docstrings of user facing functions.

Closes #951

This PR needs an in-depth review.

Checklist

  • Updated documentation
  • Ran just run-all

@signekb signekb requested a review from a team as a code owner January 21, 2025 12:03
# Set global path for the example
os.environ["SPROUT_GLOBAL"] = ".storage/"

# sp.path_package_database(package_id=1)
Copy link
Member Author

Choose a reason for hiding this comment

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

Commented out bc there is no package database in the current setup. This function will soon be removed altogether #980

Comment on lines 36 to 57
```{python}
#| output: false
#| echo: false
import os

import seedcase_sprout.core as sp
from seedcase_sprout.core.write_json import write_json

# Set global path for the example
os.environ["SPROUT_GLOBAL"] = ".storage/"

edited_properties = sp.edit_package_properties(
path=sp.path_package_properties(package_id=1),
properties=sp.PackageProperties(
name="new-package-name",
title="New Package Title",
description="This is a package description",
),
).compact_dict

write_json(edited_properties, path=sp.path_package_properties(package_id=1))
```
Copy link
Member Author

@signekb signekb Jan 21, 2025

Choose a reason for hiding this comment

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

Had to add this bc write_resource_properties() would fail otherwise bc the existing properties/data-package.json are missing required fields (name, description, title)

But it won't be showed (output and echo is false)

Comment on lines 46 to 51
Examples:
```{python}
#| output: false
#| echo: false
f = open(".storage/packages/1/resources/1/data.parquet", "x")
```
Copy link
Member Author

Choose a reason for hiding this comment

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

Added this so there was a data file to get the path for.

Comment on lines 111 to 115
```{python}
#| echo: false
f = open(".storage/packages/1/resources/1/raw/file1.csv", "x")
f = open(".storage/packages/1/resources/1/raw/file2.csv", "x")
```
Copy link
Member Author

Choose a reason for hiding this comment

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

Added these so the example wouldn't just return an empty list.

So the examples will run correctly. If `create_resource_properties` is run before `create_resource_structure` the example will fail bc the resource structure doesn't exist yet.
There's no output of this code chunk, so it's not needed.
@signekb signekb changed the title feat: ✨ add from_dict method to Properties docs: 📝 add examples to user facing functions Jan 21, 2025
By moving `write_resource_properties` down.
@signekb signekb mentioned this pull request Jan 21, 2025
2 tasks
Base automatically changed from refactor/use-properties-classes-in-userfacing-functions to main January 22, 2025 08:45
Copy link
Member

@lwjohnst86 lwjohnst86 left a comment

Choose a reason for hiding this comment

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

Nice start! I didn't finish reviewing it since the main thing to resolve here is that example code should be self-contained. It's a good exercise for us to identify what functions depend on others to run without anything else involved.

seedcase_sprout/core/create_package_structure.py Outdated Show resolved Hide resolved
seedcase_sprout/core/create_resource_properties.py Outdated Show resolved Hide resolved
seedcase_sprout/core/create_resource_structure.py Outdated Show resolved Hide resolved
_quarto.yml Outdated Show resolved Hide resolved
Comment on lines +69 to +80
# TODO: Add data to resource
# sp.write_resource_data_to_raw(
# package_id=1,
# resource_id=1,
# data="path/to/data.csv")

# sp.write_resource_parquet(
# raw_files=sp.path_resource_raw_files(package_id=1, resource_id=1),
# path=sp.path_resource_data(package_id=1, resource_id=1))

# Get the path to the resource data
# sp.path_resource_data(package_id=1, resource_id=1)
Copy link
Member Author

Choose a reason for hiding this comment

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

Some examples, such as this one, requires functionality that doesn't exist yet, so I have commented it out and added a TODO item.

Comment on lines +51 to 72
# TODO: Write package properties that passes checks
# Write package properties
# sp.write_package_properties(
# path=temp_dir / "1" / "datapackage.json",
# package_properties=sp.PackageProperties(
# title="New Package Title",
# name="new-package-name",
# description="New Description",
# ),

# Write resource properties
# sp.write_resource_properties(
# path=temp_dir / "1" / "datapackage.json",
# resource_properties=sp.ResourceProperties(
# name="new-resource-name",
# title="New resource name",
# description="This is a new resource",
# path="data.parquet",
# ),
# )
```
"""
Copy link
Member Author

Choose a reason for hiding this comment

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

Same here. Currently, we can't write resource properties on a newly created package bc the (package) properties are missing required fields.

@signekb signekb requested a review from lwjohnst86 January 24, 2025 11:42
Copy link
Member

@lwjohnst86 lwjohnst86 left a comment

Choose a reason for hiding this comment

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

Super nice!!! 🤩

seedcase_sprout/core/create_package_structure.py Outdated Show resolved Hide resolved
@lwjohnst86 lwjohnst86 merged commit 42799a0 into main Jan 24, 2025
3 checks passed
@lwjohnst86 lwjohnst86 deleted the docs/add-examples-to-user-facing-functions branch January 24, 2025 14:13
lwjohnst86 added a commit that referenced this pull request Jan 24, 2025
## Description

This PR adds docstrings to the modules with multiple/many
functions/classes.

Note that this is stacked on #982 

<!-- Select quick/in-depth as necessary -->
This PR needs an in-depth review.

## Checklist

- [X] Updated documentation
- [X] Ran `just run-all`

---------

Co-authored-by: Luke W. Johnston <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add examples to docstrings with Quarto syntax
2 participants