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

Behaviour of get_tag is different from standard expectation from get or getattr #291

Open
yarnabrina opened this issue Feb 22, 2024 · 2 comments
Labels
API design API design & software architecture enhancement Adding new functionality

Comments

@yarnabrina
Copy link
Contributor

get_tag is behaving differently from get or getattr in python. Usually they return default value provided if key is missing. In skbase, it fails even when default value is provided, unless one specifically pass an additional argument to control error behaviour.

This is not really a bug, but a behaviour that is probably not very intuitive to python users.

>>>
>>> from skbase.base import BaseObject
>>>
>>> BaseObject().get_tag("abcd")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/anirban/conda-environments/sktime/lib/python3.10/site-packages/skbase/base/_base.py", line 578, in get_tag
    return self._get_flag(
  File "/home/anirban/conda-environments/sktime/lib/python3.10/site-packages/skbase/base/_tagmanager.py", line 147, in _get_flag
    raise ValueError(f"Tag with name {flag_name} could not be found.")
ValueError: Tag with name abcd could not be found.
>>>
>>> BaseObject().get_tag("abcd", tag_value_default="pqrs")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/anirban/conda-environments/sktime/lib/python3.10/site-packages/skbase/base/_base.py", line 578, in get_tag
    return self._get_flag(
  File "/home/anirban/conda-environments/sktime/lib/python3.10/site-packages/skbase/base/_tagmanager.py", line 147, in _get_flag
    raise ValueError(f"Tag with name {flag_name} could not be found.")
ValueError: Tag with name abcd could not be found.
>>>
>>> ARIMA().get_tag("abcd", tag_value_default="pqrs", raise_error=False)
'pqrs'
>>>
yarnabrina added a commit that referenced this issue Feb 22, 2024
@fkiraly
Copy link
Contributor

fkiraly commented Feb 22, 2024

fully agree - it's in many places though, and if we change it we do not notice the places that rely on raising an error, as we switch off error conditions without switching them one.

Should we change with deprecation?

@fkiraly fkiraly added API design API design & software architecture enhancement Adding new functionality labels Feb 22, 2024
@fkiraly
Copy link
Contributor

fkiraly commented Feb 22, 2024

(in support of your argument, the behaviour is also inconsistent with get_class_tag)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API design API design & software architecture enhancement Adding new functionality
Projects
None yet
Development

No branches or pull requests

2 participants