Skip to content

Conversation

@moe-ad
Copy link
Contributor

@moe-ad moe-ad commented Dec 17, 2025

This change is needed due to ansys/pydpf-core#2816. ansys/pydpf-core#2852 also needs to be merged before this.

@codecov
Copy link

codecov bot commented Dec 17, 2025

Codecov Report

❌ Patch coverage is 50.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.05%. Comparing base (bbf9b05) to head (74e4426).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #935      +/-   ##
==========================================
- Coverage   85.12%   83.05%   -2.07%     
==========================================
  Files          52       52              
  Lines        5224     5224              
==========================================
- Hits         4447     4339     -108     
- Misses        777      885     +108     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

label_space[label_name] = int(value)
fields = self._fc.get_fields(label_space=label_space)
if isinstance(self._fc, dpf.FieldsContainer):
fields = self._fc.get_fields(label_space=label_space)
Copy link
Contributor

Choose a reason for hiding this comment

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

@moe-ad Doesn't the FieldsContainer inherit a get_entries method from the Collection class?
If not, we should add it so we can have the same API and not need such conditional tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is that FieldsContainer subclasses CollectionBase directly (a superclass of Collection) where get_entries isn't available. But _get_entries is available though, but I don't think we should be calling that directly.

@PProfizi
Copy link
Contributor

I'll merge main once #936 is ready

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