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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default values for constants in spectral indices #15

Open
remi-braun opened this issue Oct 25, 2022 · 6 comments
Open

Default values for constants in spectral indices #15

remi-braun opened this issue Oct 25, 2022 · 6 comments
Assignees
Labels
enhancement New feature or request

Comments

@remi-braun
Copy link

Hello,

Would it be possible (if useful) to have default values for constants in specified index (ie. L for SAVI) ? 馃槂
I am trying to have the minimum required intervention from the user, so it would be helpful!

@remi-braun remi-braun changed the title Default values for constants Default values for constants in spectral indices Oct 25, 2022
@remi-braun
Copy link
Author

And it would be equally awesome to avoid throwing a too generic Exception if the value is missing.
Maybe a ValueError (or a custom exception) should replace it? 馃槄

@davemlz
Copy link
Member

davemlz commented Oct 25, 2022

Hi @remi-braun,

Actually, the constant objects have default values :) please see the Constant docs. Nevertheless, this is an overall default (e.g., the most common default value of L is 1, but for SAVI is 0.5, so the user must decide).

About the Exception, could you be more specific? At the moment spyndex notifies the user that the value is missing. Or do you mean just changing the Exception type?

Exception example (as it is right now):

import spyndex

spyndex.indices.SAVI.compute()

---------------------------------------------------------------------------
Exception                                 Traceback (most recent call last)
[<ipython-input-5-bffea78d37b9>](https://localhost:8080/#) in <module>
----> 1 spyndex.indices.SAVI.compute()

2 frames
[/usr/local/lib/python3.7/dist-packages/spyndex/axioms.py](https://localhost:8080/#) in compute(self, params, **kwargs)
    155             parameters = params
    156 
--> 157         return computeIndex(self.short_name, parameters)
    158 
    159 

[/usr/local/lib/python3.7/dist-packages/spyndex/spyndex.py](https://localhost:8080/#) in computeIndex(index, params, online, returnOrigin, coordinate, **kwargs)
    219             raise Exception(f"{idx} is not a valid Spectral Index!")
    220         else:
--> 221             _check_params(idx, params, indices)
    222             result.append(eval(indices[idx]["formula"], {}, params))
    223 

[/usr/local/lib/python3.7/dist-packages/spyndex/utils.py](https://localhost:8080/#) in _check_params(index, params, indices)
     73         if band not in list(params.keys()):
     74             raise Exception(
---> 75                 f"'{band}' is missing in the parameters for {index} computation!"
     76             )

Exception: 'L' is missing in the parameters for SAVI computation!

@remi-braun
Copy link
Author

remi-braun commented Oct 25, 2022

1锔忊儯 I tried to compute a SAVI index without providing the L constant and it threw me the Exception you linked. It would be very neat that instead a SAVI index is computed with the default L value (relative to the asked index, 0.5 in this case) 馃槈

2锔忊儯 Yes, I am writing about the Exception class, it is too generic to throw it as the try except cannot be specific after that (except Exception is way too generic). Throwing a Value Error would be more appropriate, or even better, a custom Spyndex Exception that would allow me to catch this exception properly 馃憤

@davemlz
Copy link
Member

davemlz commented Oct 25, 2022

  1. I like this idea! But I don't want to leave it as default since the idea of these indices is that people should use the value that they need for their application (even if they don't provide the parameters). I will probably add an use_default argument to the computation. I will add this to an improvement Issue soon :)
  2. Fair enough! :) I will also add it soon!

Thanks a lot, @remi-braun!

@remi-braun
Copy link
Author

Thanks a lot 馃槈

As promised, I am trying to use spyndex in the EOReader lib, this may lead to several issues and questions, please be patient with me haha 馃檹

@davemlz
Copy link
Member

davemlz commented Oct 29, 2022

Haha sure!

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