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

Text.style type Deprecation warning #2286

Merged
merged 2 commits into from
Jan 8, 2024
Merged

Text.style type Deprecation warning #2286

merged 2 commits into from
Jan 8, 2024

Conversation

ndonkoHenri
Copy link
Contributor

@ndonkoHenri ndonkoHenri commented Dec 30, 2023

Close #2284

The warning message is shown only when setting the style as below (after instance creation):

import flet as ft
t = ft.Text(
    "Test!",
    # style=ft.TextThemeStyle.HEADLINE_SMALL,
)
t.style = ft.TextThemeStyle.HEADLINE_SMALL

When setting it at the Text instance creation, the warning is weirdly not shown.

@FeodorFitsner
Copy link
Contributor

Ha ha, I've asked ChatGPT about that and got this:

However, certain types of warnings, such as DeprecationWarning, are ignored by default.

If you’re not seeing the warnings, it’s possible that the warning is being suppressed. This can happen if the warning filter, which is a sequence of matching rules and actions, is set to ignore the type of warning you’re trying to issue.

By adding a filter I can now see DeprecationWarnings:

import warnings

import flet as ft

warnings.simplefilter("always", DeprecationWarning)

t = ft.Text(
    "Test!",
    style=ft.TextThemeStyle.HEADLINE_SMALL,
)

I'm not sure I like this Python default bahavior. I don't think someone will guess to enable warnings.

If DeprecationWarning category is removed from warn() method the warning is shown as UserWarning in all cases. Shall we remove it then?

@ndonkoHenri
Copy link
Contributor Author

If DeprecationWarning category is removed from warn() method the warning is shown as UserWarning in all cases. Shall we remove it then?

I tried your suggestion, and two warnings (instead of just one) are now shown:

/Users/ndonkohenri/PycharmProjects/flet-dev/flet/sdk/python/packages/flet-core/src/flet_core/text.py:172: UserWarning: If you wish to set the TextThemeStyle, use `Text.theme_style` instead. The `Text.style` property should be used to set the TextStyle only.
  self.style = style
/Users/ndonkohenri/PycharmProjects/flet-dev/flet/sdk/python/playground/test-changes.py:8: UserWarning: If you wish to set the TextThemeStyle, use `Text.theme_style` instead. The `Text.style` property should be used to set the TextStyle only.
  t.style = ft.TextThemeStyle.HEADLINE_SMALL

This is not a problem though. Should I nevertheless move on commiting the removed DeprecationWarning.

@FeodorFitsner
Copy link
Contributor

Two warnings is correct behaviour as the 1st call is in constructor and 2nd in property setter.
Yes, let's remove DeprecationWarning.

@FeodorFitsner FeodorFitsner merged commit 9de53ea into main Jan 8, 2024
2 checks passed
@ndonkoHenri ndonkoHenri deleted the style-deprecation branch January 8, 2024 22:26
zrr1999 pushed a commit to zrr1999/flet that referenced this pull request Jul 17, 2024
* initial commit

* remove DeprecationWarning
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

Successfully merging this pull request may close these issues.

Add deprecation warning in Text.style setter
2 participants