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

Some refactoring suggestions #11

Open
rthedin opened this issue Jul 5, 2022 · 0 comments
Open

Some refactoring suggestions #11

rthedin opened this issue Jul 5, 2022 · 0 comments
Labels
enhancement New feature or request

Comments

@rthedin
Copy link
Collaborator

rthedin commented Jul 5, 2022

I have some suggestions that I think it will improve code maintenance and debugging. None of this is exactly needed at this point, but if time becomes available, it would be nice to have.

  1. Exception handling. When the code checks if a file already exists in order to load it (instead of downloading or computing it again), it is done in a try/except way. The exceptions are also caught generically, on a catch-all umbrella (e.g. except Exception as _).

    SSRS/ssrs/simulator.py

    Lines 161 to 165 in 1cda662

    try:
    slope = self.get_terrain_layer('Slope')
    except Exception as _:
    elev = self.get_terrain_elevation()
    slope = compute_slope_degrees(elev, self.resolution)

    I think we should first check if the file exists, then read it if it does. Only then, we can handle exceptions if something goes wrong. We should not fall into an exception if the file does not exist, since that will happen the first time running it. These functions should return some False/True, instead of carrying over a generic exception. In some other instances, computations are performed within a try/except block and I think that should really be removed. When something goes bad, the actual exception is never given to the user.

  2. We should really consider moving grid data to an xarray format. The rasterio operations can probably be handled by rioxarray . Problems with orientation of aspect and wind direction conventions are handled ad-hoc. For example, this should be removed:

    wdir_sx = (np.mean(wdirn)+90)%360 # wdir for sx due to weird convention

    w0 = wspeedAtRefHeight * np.sin(np.deg2rad(slope)) * np.cos(np.deg2rad(((-np.mean(wdirn)+90)%360)-aspect))

  3. The simulator class has too many methods that should not be there. For example, all the plotting routines should be moved to another file/class.

  4. kwargs keywords should be added to plotting functions, which are then passed to the actual pcolormesh calls.

@rthedin rthedin added the enhancement New feature or request label Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants