search

stfc / PSyclone

Domain-specific compiler and code transformation system for Finite Difference/Volume/Element Earth-system models in Fortran
BSD 3-Clause "New" or "Revised" License
107 stars 29 forks source link

Add `is_character` method for `DataNode` #2753

Closed jwallwork23 closed 3 weeks ago

jwallwork23 commented 1 month ago

Partially addresses #2717.

This PR merges one of two functions from the types module of PSyACC into PSyclone.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.86%. Comparing base (33ff7e8) to head (87c257b). Report is 16 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2753 +/- ## ======================================= Coverage 99.86% 99.86% ======================================= Files 354 354 Lines 48967 48973 +6 ======================================= + Hits 48903 48909 +6 Misses 64 64 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

arporter commented 3 weeks ago

Could I be a bit cheeky and request a fix for a docstring formatting error from a previous PR? Building the Ref. Guide now gives:

PSyclone/src/psyclone/psyir/nodes/assignment.py:docstring of psyclone.psyir.nodes.Assignment.is_literal_assignment:3: WARNING: Definition list ends without a blank line; unexpected unindent.

which is new!

jwallwork23 commented 3 weeks ago

Could I be a bit cheeky and request a fix for a docstring formatting error from a previous PR? Building the Ref. Guide now gives:

PSyclone/src/psyclone/psyir/nodes/assignment.py:docstring of psyclone.psyir.nodes.Assignment.is_literal_assignment:3: WARNING: Definition list ends without a blank line; unexpected unindent.

which is new!

Ah good spot. Apologies, should be fixed in f2952478fc01c8e8d78cad0544dc43efd562054a.

sergisiso commented 3 weeks ago

Ah good spot. Apologies, should be fixed in https://github.com/stfc/PSyclone/commit/f2952478fc01c8e8d78cad0544dc43efd562054a.

Ups, I missed this on my review. I think the issue comes from missing the starting colon in the :returns: and :rtype: tags. It is also wrong in the method above.

With this fixed I don't think the backslash is necessary.

jwallwork23 commented 3 weeks ago

Ups, I missed this on my review. I think the issue comes from missing the starting colon in the :returns: and :rtype: tags. It is also wrong in the method above.

With this fixed I don't think the backslash is necessary.

Oh okay, I had just copied the syntax from the method above it. Fixed for both methods in 2c783a787f0b8daacf14f385b75e1d74290e0f65.

jwallwork23 commented 3 weeks ago

This is ready to merge now @jwallwork23 . Note that I ended up changing the "raise Exception" to a "raise ValueError" to avoid the pylint "broad-exception-raised" issue.

I also built the multiple documentations to verify the docstring warnings are gone.

Great, thanks @sergisiso!