Allow deepcopy of fparser tree

[schreiberx]

Does what's in the title.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.14%. Comparing base (84a4b1d) to head (ed6decb).

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #453   +/-   ##
=======================================
  Coverage   92.14%   92.14%           
=======================================
  Files          86       86           
  Lines       13834    13840    +6     
=======================================
+ Hits        12747    12753    +6     
  Misses       1087     1087           

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

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

[arporter]

Looks good @schreiberx apart from the lack of testing ;-)
Needs a few more comments/doc strings too.
Looking at the docs for pickle, I see that json would probably enable us to make portable serialisations but I'm not sure how useful that would be at this stage. Could be good for getting reproducers easily in the future (e.g. if a user has a failure) but that said, a Fortran fragment has always been fine so far.

[schreiberx]

Back to @arporter

[arporter]

Hi @schreiberx, please could you address the Black error(s) and missing coverage? Thanks :-)

[schreiberx]

Back to @arporter

[arporter]

Thanks Martin. Almost there now bar some tidying.
That's one of the advantages of coverage testing - it makes you realise when you've added something unnecessary (or that things aren't working the way you thought).
Just a reminder - please leave the conversations for me to resolve. That way it's easier for me to make sure everything has been done.

[schreiberx]

Back to @arporter

[arporter]

Thanks Martin, I like the new/extended tests. Probably worth putting in a bit more Fortran into the example code in both just to exercise things a bit more.

However, @sergisiso raises a good point about the symbol table. Currently, there is a single, global fparser.two.symbol_table.SYMBOL_TABLES object that holds information on all scoping regions encountered during a parse. We made it like this so that it is easily available whenever needed during fparser's complex, recursive execution. We need to think about whether we pickle it separately or whether we can somehow attach it to the parse tree.

[schreiberx]

Thanks Martin, I like the new/extended tests. Probably worth putting in a bit more Fortran into the example code in both just to exercise things a bit more.

However, @sergisiso raises a good point about the symbol table. Currently, there is a single, global fparser.two.symbol_table.SYMBOL_TABLES object that holds information on all scoping regions encountered during a parse. We made it like this so that it is easily available whenever needed during fparser's complex, recursive execution. We need to think about whether we pickle it separately or whether we can somehow attach it to the parse tree.

fparser.two.symbol_table.SYMBOL_TABLES is a separate thing and I don't see this to be related to this deepcopy issue. Maybe open another one?

[schreiberx]

Back to @arporter

[arporter]

Thanks for that Martin. I'm reasonably happy now. Note that the parser does populate a tree of symbol tables (albeit only for intrinsic types currently) as it goes. However, we don't currently attempt to make any use of those symbol tables. Therefore, if @sergisiso is happy, I'm happy that this be merged as it is a valuable step towards being able to do caching. We need to think through how we're going to handle the symbol table(s) separately. Ideally, the tables would go into the same cache as the parse tree that they are associated with.

I have to admit that this has made me uncomfortable about the decision to have a single, global SYMBOL_TABLES object. Probably, as a collaborator suggested a few years ago (#191), the whole structure of fparser could do with being reworked to get rid of global state. This would then be a more natural fit for pickling.

[sergisiso]

@arporter @schreiberx Yes I am happy. I forgot that in fparser the symbol_table is only a supporting state to recurse down the tree, but not part of the output (all declaration info is in the parsetree). So I agree should not be part of the cache, sorry for the confusion.