ErikNixdorf / sbat

The Repo for the Surface Water Balance Analysis Tool
BSD 3-Clause "New" or "Revised" License
3 stars 0 forks source link

Refactoring SBAT Modul Class #41

Closed ErikNixdorf closed 1 year ago

ErikNixdorf commented 1 year ago
          Okay this means we have to configure the logger in `main()`. This will ensure the configuration is read before so we can access the output path from `sbat.config`, and running `sbat.main()` from another module (`apply_sbat.py`) will work. 

However, the other submodules will then have problems to access the logger. Maybe the logger can be stored inside the sbat class, the logger could also be created by an external function. I need to think about this, maybe it is smarter to refactor the sbat class also in regard to the testing, where we have to find a way to test different configurations.

I have a full schedule this week and can likely work on this from Thursday on.

Originally posted by @MarcoHannemann in https://github.com/ErikNixdorf/sbat/issues/40#issuecomment-1493946537

ErikNixdorf commented 1 year ago

@MarcoHannemann Can I assign it to you?

MarcoHannemann commented 1 year ago

Yes, I am already working on it (class_refactoring). I redesigned the Model class, but there are still some todos left; some minor things and the relocation of the logger. After that I will see if a testing framework with different configurations can be efficiently added.

Before I will open a PR and we can discuss the changes.

ErikNixdorf commented 1 year ago

ok, alright. Thx for your support. I am curious to see it. I will continue working on test examples and plotting capabilities

MarcoHannemann commented 1 year ago

What is the idea behind the apply_sbat.py script? Why not just call python sbat/sbat.py from the command line, instead of accessing the sbat.main() function?

ErikNixdorf commented 1 year ago

It tests whether it works to import and access sbat from another script. Typical scenario is that a user writes some user code and loads sbat and may calls only some if its functionality. To test it, I use apply_sbat.py

MarcoHannemann commented 1 year ago

I see. At the moment this script is not working for me, it's the first time I tested it.

Traceback (most recent call last):
  File "/home/hannemam/Projects/sbat/apply_sbat.py", line 9, in <module>
    model=sbat.main(output=True)
AttributeError: module 'sbat' has no attribute 'main'

What works is

from sbat import sbat
sbat.main()

Do you know what could be the reason for this behavior?

MarcoHannemann commented 1 year ago

Any updates on my problem with apply_sbat.py? Also doesn't work from terminal:

(sbat) hannemam@modmon186l:~/Projects/sbat$ python apply_sbat.py 
Traceback (most recent call last):
  File "/home/hannemam/Projects/sbat/apply_sbat.py", line 9, in <module>
    model=sbat.main(output=True)
AttributeError: module 'sbat' has no attribute 'main'
ErikNixdorf commented 1 year ago

well, if I changed line 5 to from sbat import sbat , then it works from terminal but then it does not work calling it from the spyder ide anymore. I assume it is a root path problem. What do you think?

MarcoHannemann commented 1 year ago

Makes sense - is it possible that you have set the project root directory to sbat/sbat/ within Spyder? Logically, within the main directory of the repo, we would import from sbat, as it is a downward directory. from sbat import main should only work if we are located in the same directory as sbat.py. That explains the error - Python finds an sbat, but thats the folder, not the script, so the main function cannot be found.

ErikNixdorf commented 1 year ago

I suggest we just remove apply_sbat.py from the repo. I can keep it locally and anybody should be able to write this few lines by himself. Do you agree?

MarcoHannemann commented 1 year ago

Agreed, let's remove it from branch pytest so merging will be easier later.

MarcoHannemann commented 1 year ago

Refactoring addressed in #46, further discussion of logger implementation in #52.