Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Importing fipy raises error when testing with pytest in vscode (--cache-clear) #895

Open
martinwk opened this issue Dec 21, 2022 · 7 comments

Comments

@martinwk
Copy link

I am using fipy to run some simple 1D convection-reaction model. This is part of a larger model suite and as such it needs to be automatically (unit) tested. We do that with pytest. This works well, unless I run pytest with --cache-clear argument.

This is very annoying because my IDE (vscode) run the tester with this command. It seems like the --cache-clear argument is swallowed by the argument parser of fipy which raises the "BadOption" error. I have the feeling it has to do with the import * in the fipy files which is as far as I understood, not adviced to (according to PEP8 I believe). Any idea for a workaround?

It returns the error:

During handling of the above exception, another exception occurred:
..\..\..\Miniconda3\envs\aquapriori_api\lib\importlib\__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1030: in _gcd_import
    ???
<frozen importlib._bootstrap>:1007: in _find_and_load
    ???
<frozen importlib._bootstrap>:986: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:680: in _load_unlocked
    ???
..\..\..\Miniconda3\envs\aquapriori_api\lib\site-packages\_pytest\assertion\rewrite.py:168: in exec_module
    exec(co, module.__dict__)
tests\test_ac_functions.py:2: in <module>
    from AquaPrioriAPI import acfunctions, generic_functions
AquaPrioriAPI\acfunctions.py:8: in <module>
    from wt_process_models.gac import isotherm_functions, process_models
..\wt_process_models\wt_process_models\gac\process_models.py:11: in <module>
    import fipy
..\..\..\Miniconda3\envs\aquapriori_api\lib\site-packages\fipy\__init__.py:40: in <module>
    from fipy.boundaryConditions import *
..\..\..\Miniconda3\envs\aquapriori_api\lib\site-packages\fipy\boundaryConditions\__init__.py:3: in <module>
    from fipy.boundaryConditions.fixedFlux import *
..\..\..\Miniconda3\envs\aquapriori_api\lib\site-packages\fipy\boundaryConditions\fixedFlux.py:6: in <module>
    from fipy.boundaryConditions.boundaryCondition import BoundaryCondition
..\..\..\Miniconda3\envs\aquapriori_api\lib\site-packages\fipy\boundaryConditions\boundaryCondition.py:6: in <module>
    from fipy.variables.variable import Variable
..\..\..\Miniconda3\envs\aquapriori_api\lib\site-packages\fipy\variables\__init__.py:1: in <module>
    from fipy.variables.variable import *
..\..\..\Miniconda3\envs\aquapriori_api\lib\site-packages\fipy\variables\variable.py:19: in <module>
    class Variable(object):
..\..\..\Miniconda3\envs\aquapriori_api\lib\site-packages\fipy\variables\variable.py:46: in Variable
    if parser.parse("--cache", action="store_true"):
..\..\..\Miniconda3\envs\aquapriori_api\lib\site-packages\fipy\tools\parser.py:45: in parse
    (options, args) = tmpparser.parse_args(sysargs)
..\..\..\Miniconda3\envs\aquapriori_api\lib\optparse.py:1389: in parse_args
    self.error(str(err))
..\..\..\Miniconda3\envs\aquapriori_api\lib\optparse.py:1569: in error
    self.exit(2, "%s: error: %s\n" % (self.get_prog_name(), msg))
..\..\..\Miniconda3\envs\aquapriori_api\lib\optparse.py:1559: in exit
    sys.exit(status)
E   SystemExit: 2

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _  

self = <optparse.OptionParser object at 0x0000013CED08B7C0>, status = 2, msg = 'pytest: error: no such option: --cache-clear\n'        

    def exit(self, status=0, msg=None):
        if msg:
            sys.stderr.write(msg)
>       sys.exit(status)
E       SystemExit: 2

..\..\..\Miniconda3\envs\aquapriori_api\lib\optparse.py:1559: SystemExit

pytest 7.2
python 3.9
fipy 3.4.3

@wd15
Copy link
Contributor

wd15 commented Dec 21, 2022

@martinwk: fipy for sure shouldn't be importing with import *. It was all written a long time ago! As a hack, maybe alter the sysargs option here to remove the offending argument. Something like that might work as a work around.

@guyer
Copy link
Member

guyer commented Dec 22, 2022

This has nothing to do with import *. If we got rid of all instances of import *, then we would have an explicit from fipy.variables.variable import Variable someplace in the course of import fipy. This would trigger the same error.

The actual source of the problem is that pytest appears to have monkey-patched optparse. FiPy is trying to check its command-line options and pytest is interfering (cf. "pytest: error: no such option: --cache-clear"). fipy.tools.parser.parse() explicitly ignores options it doesn't know; pytest is somehow forcing optparse to look at options that were not passed to it.

As to import *, as @wd15 says, it was all written a long time ago and, at least at the time, FiPy was hardly the only package that flattened its public API this way. I would argue that what we are doing falls under "There is one defensible use case for a wildcard import, which is to republish an internal interface as part of a public API" from PEP8.

@guyer
Copy link
Member

guyer commented Dec 22, 2022

Actually, I should clarify that I don't know that pytest did the monkey-patching. In fact, if I had to guess, vscode is probably the culprit.

@martinwk
Copy link
Author

Thanks for your quick reply.

@guyer It is not the IDE but pytest itself since running the above mentioned command in a cli yields the same error.

@guyer I guess you are right about that import* not being the problem here because an explicit import will trigger the same code which will yield the same problem. (And whether it is pep compliant is not too relevant as long as things work as expected I would say)

I understand the code is written along time ago with the legacy artefacts that this brings. No intrinsic problem here. That is why I asked for hints for a workaround and not a fix from your side (especially since this seems to be an edge case).

@wd15 thanks for your suggestion for a workaround. Do you suggest to fork the code and alter the code there? Of should I something similar in my code to swallow the sysarg before importing fipy? In his regard, i wondered why fipy is parsing sysargs anyway when the package is imported in a python file . Seems to me that this is only necessary in some cases.

@wd15
Copy link
Contributor

wd15 commented Dec 27, 2022

@wd15 thanks for your suggestion for a workaround. Do you suggest to fork the code and alter the code there?

That's what I was thinking, but no idea if it will fix your problems. Jon seems to suggest that something else is going on. Trying won't hurt, however.

Of should I something similar in my code to swallow the sysarg before importing fipy?

That would be preferable to hacking FiPy from your perspective. Try and see what happens.

In his regard, i wondered why fipy is parsing sysargs anyway when the package is imported in a python file . Seems to me that this is only necessary in some cases.

Again, it's the legacy thing. We might deal with this in a different way now. We wanted to able to run

  $ python fipycode.py --pysparse 

for example, and use the pysparse solver suite or any one of a myriad of other choices. Nowadays, we'd probably go with a config file or some other more consistent system that doesn't turn FiPy into a command line tool. However, as @guyer pointed out, it might not be FiPy causing the issue in your case.

@martinwk
Copy link
Author

Ok thanks for confirming. I will try after the Christmas holidays.

@guyer
Copy link
Member

guyer commented Jan 3, 2023

It is not the IDE but pytest itself since running the above mentioned command in a cli yields the same error.

That's helpful to know, thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants