Add VertexField to support field data at vertices

[swapneelap]

No description provided.

[github-actions]

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 the Field 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 the from_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: @classmethod

  • relevant 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. @Swapneel

How to use

Instructions

To invoke the PR-Agent, add a comment using one of the following commands:
/review: Request a review of your Pull Request.
/describe: Update the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback.
/ask <QUESTION>: Ask a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.
/add_docs: Generate docstring for new components introduced in the PR.
/generate_labels: Generate labels for the PR based on the PR's contents.
see the tools guide for more details.

To edit any configuration parameter from the configuration.toml, add --config_path=new_value.
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, add a /config comment.

[codecov-commenter]

Codecov Report

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 ⚠️
discretisedfield/field.py	73.91%	6 Missing ⚠️
discretisedfield/cell_field.py	98.59%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #511      +/-   ##
==========================================
- Coverage   93.49%   91.47%   -2.03%     
==========================================
  Files          28       30       +2     
  Lines        3027     3154     +127     
==========================================
+ Hits         2830     2885      +55     
- Misses        197      269      +72     

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

[swapneelap]

@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 😃

[samjrholt]

As discussed, we should use abstractmethod rather than overwriting the base class

• mpl
• to_xarray
• from_xarray
• _hv_key_dims

[samjrholt]

Are we sure about the name? (Just checking 👍 )

[swapneelap]

I am open to ideas. I am OK with this one as well. Thoughts @lang-m