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

Fixed bug related to missing_value for android and linux #1913

Merged

Conversation

LunaMeadows
Copy link
Contributor

@LunaMeadows LunaMeadows commented Apr 29, 2023

For linux, I passed the missing_value from self.interface.missing_value from Tree to the sourcetreemodel. I did this because self.instance is not available inside the sourcetreemodel but the value is needed to be able to solve the bug.

For android, I simply added the self.interface.missing_value as the default option for the getattr.

On android the app will crash if values are not provided for all headings, on linux it will cause an error in the logs.

Fixes #1879

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@@ -15,6 +15,7 @@ def create(self):
self.store = SourceTreeModel(
[{"type": str, "attr": a} for a in self.interface._accessors],
is_tree=is_tree,
missing_value=getattr(self.interface, "missing_value", None),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use getattr here? Unless I'm missing something, missing_value should always be defined on the interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not at my computer to confirm fully but I believe the issue was the Tree interface did not have the missing_value. If you are able to look at the first round of checks it should show which interface did have it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah - that makes sense. That's an API inconsistency that should probably be resolved...

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The missing_value handling on tree is something we need to backfill for tree, but otherwise, this looks great! Thanks!

@@ -15,6 +15,7 @@ def create(self):
self.store = SourceTreeModel(
[{"type": str, "attr": a} for a in self.interface._accessors],
is_tree=is_tree,
missing_value=getattr(self.interface, "missing_value", None),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah - that makes sense. That's an API inconsistency that should probably be resolved...

@freakboy3742 freakboy3742 force-pushed the android_table_missing_value_#1879 branch from ff3186f to a49ef11 Compare May 2, 2023 06:07
@freakboy3742 freakboy3742 merged commit f4184a1 into beeware:main May 2, 2023
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.

Android table missing_value
2 participants