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

✨ Add get_needs_view function to public API #1277

Closed
wants to merge 1 commit into from

Conversation

chrisjsewell
Copy link
Member

@chrisjsewell chrisjsewell commented Sep 5, 2024

This allows users to access to the (immutable) resolved needs during the write/analysis phase (for example https://github.com/useblocks/sphinx-modeling, cc @ubmarco, also see #1264 for more info)

To ensure it is not misused, checks are now put in-place to disallow modification of needs after the needs have been resolved.
This actually surfaced a few issues where the resolved needs were mistakenly accessed, instead of the unresolved (mutable) ones.

The other reason for doing this, is that if we "know" we are only accessing immutable needs, then we will be able to improve filtering performance, by performing lazy indexing etc (see #1281)

Copy link

codecov bot commented Sep 5, 2024

Codecov Report

Attention: Patch coverage is 75.86207% with 7 lines in your changes missing coverage. Please review.

Project coverage is 86.95%. Comparing base (4e10030) to head (0975799).
Report is 54 commits behind head on master.

Files with missing lines Patch % Lines
sphinx_needs/data.py 63.63% 4 Missing ⚠️
sphinx_needs/functions/common.py 33.33% 2 Missing ⚠️
sphinx_needs/api/need.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1277      +/-   ##
==========================================
+ Coverage   86.87%   86.95%   +0.07%     
==========================================
  Files          56       60       +4     
  Lines        6532     6978     +446     
==========================================
+ Hits         5675     6068     +393     
- Misses        857      910      +53     
Flag Coverage Δ
pytests 86.95% <75.86%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

"""
if not self.needs_is_post_processed:
from sphinx_needs.directives.need import post_process_needs_data
Copy link
Member

Choose a reason for hiding this comment

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

Can you raise an exception if not in the right phase?

@chrisjsewell
Copy link
Member Author

This has been subsumed by #1281

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.

2 participants