Open swapneelap opened 11 months ago
Attention: 73 lines
in your changes are missing coverage. Please review.
Comparison is base (
30bf3c7
) 93.49% compared to head (43da142
) 91.47%.
Files | Patch % | Lines |
---|---|---|
discretisedfield/vertex_field.py | 33.33% | 66 Missing :warning: |
discretisedfield/field.py | 73.91% | 6 Missing :warning: |
discretisedfield/cell_field.py | 98.59% | 1 Missing :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@lang-m the hvplot should be working with the VertexField
now. Play with it if you find some time and let me know if something looks odd.
PS: Thank you for setting up hv
in a way that small modification to the to_xarray
fixed the whole plotting for VertexField
๐
PR Analysis
๐ฏ Main theme: Addition of a new class
VertexField
to support field data at vertices.๐ PR summary: This PR introduces a new class
VertexField
that extends theField
class. The new class includes methods for representation, calling, differentiation, integration, line creation, item retrieval, conversion to VTK, mpl, hv, and xarray formats, and class method for creation from xarray. However, most of these methods are not yet implemented.๐ Type of PR: Enhancement
๐งช Relevant tests added: No
โฑ๏ธ Estimated effort to review [1-5]: 3, because the PR introduces a new class with several methods, but most of them are not implemented yet, which makes it hard to evaluate the overall functionality.
๐ Security concerns: No security concerns found
PR Feedback
๐ก General suggestions: The PR introduces a new class with several methods, but most of them are not implemented yet. It would be beneficial to implement these methods or at least provide a clear plan for their implementation. Also, it's recommended to add relevant tests to ensure the correct functionality of the new class.
๐ค Code feedback:
relevant file:
discretisedfield/vertex_field.py
suggestion: Implement the__call__
method or provide a clear plan for its implementation. This method is crucial for the functionality of the class as it defines how an instance of the class responds to function calls. [important] relevant line: def call(self, point):relevant file:
discretisedfield/vertex_field.py
suggestion: Implement the__getitem__
method or provide a clear plan for its implementation. This method is important for accessing elements of the class instance. [important] relevant line: def getitem(self, item):relevant file:
discretisedfield/vertex_field.py
suggestion: Implement thefrom_xarray
class method or provide a clear plan for its implementation. This method is important for creating an instance of the class from an xarray. [important] relevant line: @classmethodrelevant file:
discretisedfield/vertex_field.py
suggestion: Remove or address the TODO comment about reimplementing the_as_array
functions. Leaving TODO comments in the code can lead to confusion and clutter. [medium] relevant line: # TODO: reimplement the _as_array functions. @SwapneelHow to use
Instructions