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

Document basis #274

Merged
merged 32 commits into from
Dec 12, 2024
Merged

Document basis #274

merged 32 commits into from
Dec 12, 2024

Conversation

BalzaniEdoardo
Copy link
Collaborator

@BalzaniEdoardo BalzaniEdoardo commented Dec 3, 2024

Restructured the background documentation for basis:

  • Folder dedicated to basis in background
  • Readme with high level overview and a table showcasing all basis types.
  • Links to background on 1d and multi-d basis

This PR follows #273

@BalzaniEdoardo BalzaniEdoardo marked this pull request as ready for review December 4, 2024 22:01
@codecov-commenter
Copy link

codecov-commenter commented Dec 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.10%. Comparing base (bf87623) to head (afdf8a8).
Report is 226 commits behind head on development.

Additional details and impacted files
@@               Coverage Diff               @@
##           development     #274      +/-   ##
===============================================
- Coverage        97.27%   96.10%   -1.17%     
===============================================
  Files               25       34       +9     
  Lines             2199     2491     +292     
===============================================
+ Hits              2139     2394     +255     
- Misses              60       97      +37     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@billbrod billbrod mentioned this pull request Dec 6, 2024
@billbrod
Copy link
Member

billbrod commented Dec 6, 2024

Things I noticed in #273 that should be part of this PR :

  • want to be consistent in how we refer to eval / conv. Are they "types"? "modes"? "categories"? should "eval" be capitalized? in quotes?

EDIT: We're going to refer to them as "categories", and generally use Eval / Conv to refer to them.

Copy link
Member

@billbrod billbrod left a comment

Choose a reason for hiding this comment

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

Some small changes that can be committed here, and then I pushed some tweaks to the wording. If you think my changes look good, then I'm good to go!

The only thing I did potentially worth discussing is that I added a bit to the top of the composite basis page trying to give a quick easy-to-understand summary of additive vs. multiplicative, so double-check that's accurate!

path.mkdir(parents=True, exist_ok=True)

if path.exists():
fig.savefig(path / "plot_01_1D_basis_function.svg")


print(path.resolve(), path.exists())
Copy link
Member

Choose a reason for hiding this comment

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

is this section still necessary (with the plot_directive)? and, regardless, we probably don't need to print out the path right, that's just for internal debugging

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed it!

@@ -304,7 +304,7 @@ if root:
path = Path(root) / "html/_static/thumbnails/background"
# if local store in ../_build/html/...
else:
path = Path("../_build/html/_static/thumbnails/background")
path = Path("../../_build/html/_static/thumbnails/background")
Copy link
Member

Choose a reason for hiding this comment

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

same point about whether we need this anymore

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed this too

docs/background/basis/README.md Outdated Show resolved Hide resolved
@BalzaniEdoardo BalzaniEdoardo merged commit 5f66190 into development Dec 12, 2024
13 checks passed
@BalzaniEdoardo BalzaniEdoardo deleted the document_basis branch December 12, 2024 13:52
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.

3 participants