(Closes #225) Add .get_name() to Function_Stmt and Entity_Decl#323
(Closes #225) Add .get_name() to Function_Stmt and Entity_Decl#323arporter merged 6 commits intostfc:masterfrom
.get_name() to Function_Stmt and Entity_Decl#323Conversation
Codecov Report
@@ Coverage Diff @@
## master #323 +/- ##
=======================================
Coverage 91.33% 91.33%
=======================================
Files 36 36
Lines 13159 13163 +4
=======================================
+ Hits 12019 12023 +4
Misses 1140 1140
Continue to review full report at Codecov.
|
|
Many thanks for this enhancement. Note that one of the conditions of code review (https://github.com/stfc/fparser/wiki/CodeReview) is that there is full test coverage of any modified code. The tests themselves need to be in files named for the classes/rules to which they apply. Therefore, the one for Function_Stmt is good but the other needs to be moved. If there are is no such file yet (and for many classes there aren't) then please copy one of the existing ones and add your tests. Please also search the massive fortran2003_test.py and move any tests of that class into the new file too. |
|
Done! I have also added tests for the |
|
I notice that some classes use Probably something for a different PR, but is there any interest in converting all the accessor methods for the various |
.get_name() to Function_Stmt and Entity_Decl.get_name() to Function_Stmt and Entity_Decl
arporter
left a comment
There was a problem hiding this comment.
Great work, thanks for doing all of that.
You'll see I've requested (brief) docstrings for the tests and they also need the f2003_parser fixture setup. Currently we seem to do that on a test-by-test basis so I'm happy for you to copy that although ultimately we really need to add an autouse for the files where it makes sense.
I've also updated the PR title so that #225 will get closed when this is merged.
|
All done! @arporter ready for review I've added
Just to note that the |
|
|
||
| :returns: the program name as a `Name` class | ||
| :rtype: `Name` | ||
| :returns: the program name as a :py:class:`Name` class |
There was a problem hiding this comment.
There's no need to duplicate the type specification in the :returns: field but it's not a biggie.
arporter
left a comment
There was a problem hiding this comment.
All requested changes have been made (thanks).
Tests now pass in isolation.
No documentation update required.
Issue number not mentioned anywhere.
I will proceed to merge this.
* master: pr #322. Updated changelog ready for merge to master. #322 try quoting python versions in GHA yml #322 updates for review #323 update changelog Use Sphinx roles to some docstrings Add some test docstrings Move entity decl test under f2003 directory Auto-use parser creation fixtures Add more tests for `get_name` methods Add `.get_name()` to `Function_Stmt` and `Entity_Decl`
* master: pr #322. Updated changelog ready for merge to master. #322 try quoting python versions in GHA yml #322 updates for review #323 update changelog Use Sphinx roles to some docstrings Add some test docstrings Move entity decl test under f2003 directory Auto-use parser creation fixtures Add more tests for `get_name` methods Add `.get_name()` to `Function_Stmt` and `Entity_Decl`
* master: pr #322. Updated changelog ready for merge to master. #322 try quoting python versions in GHA yml #322 updates for review #323 update changelog Use Sphinx roles to some docstrings Add some test docstrings Move entity decl test under f2003 directory Auto-use parser creation fixtures Add more tests for `get_name` methods Add `.get_name()` to `Function_Stmt` and `Entity_Decl` #322 rm py2-specific code from Fortran2003.py #322 fix-up coverage and tidy some pylint issues #322 fix links and formatting in dev guide #322 update dev guide #322 remove the make_clean_tmpfile utility and associated tests #307 change GHA to run with python 3.6, 3.8 and 3.10
Fixes #225
Probably I've not added the tests to the best location, so I'm happy to move them if there's somewhere better