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

Fix Dataset._get_field_info() error message for unfindable field #2775

Merged
merged 2 commits into from
Aug 6, 2020

Conversation

neutrinoceros
Copy link
Member

@neutrinoceros neutrinoceros commented Jul 23, 2020

PR Summary

Currently, when Dataset._get_field_info() is fed bad data (unfindable field), it raises an error which contains a seemingly random ftype value, unrelated to external input.

This fixes it

import yt
ds = yt.testing.fake_amr_ds()
ds.r["hello"]
old >> YTFieldNotFound: Could not find field '('stream', 'hello')' in AMRGridData.
new >> YTFieldNotFound: Could not find field ('unknown', 'hello') in AMRGridData.

PR Checklist

  • pass flake8 yt/
  • pass isort . --check --diff
  • pass black --check yt/

@neutrinoceros neutrinoceros added bug enhancement Making something better api-consistency naming conventions, code deduplication, informative error messages, code smells... labels Jul 23, 2020
@munkm munkm removed the enhancement Making something better label Jul 23, 2020
@munkm
Copy link
Member

munkm commented Jul 23, 2020

Hey I'm just moving the enhancement label on this because when we do our release notes this should be categorized as a bug.

@neutrinoceros
Copy link
Member Author

@FIdo test this please

@munkm
Copy link
Member

munkm commented Jul 23, 2020

@yt-fido test this please

1 similar comment
@neutrinoceros
Copy link
Member Author

@yt-fido test this please

…guess the ftype, the error message shows the user input instead of the latest internal attempt
@neutrinoceros
Copy link
Member Author

quick note that once this is approved and merged, we should take another look at broken examples in #2723, two of which are failing with YTFieldNotFound or related errors.

Copy link
Member

@matthewturk matthewturk left a comment

Choose a reason for hiding this comment

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

This looks good to me. Some of this logic could be avoided if we did not allow for ambiguous field access.

@cphyc cphyc merged commit bf97f0a into yt-project:master Aug 6, 2020
@neutrinoceros neutrinoceros deleted the refactor_get_field_info branch August 6, 2020 15:51
@neutrinoceros neutrinoceros added UX user-experience and removed api-consistency naming conventions, code deduplication, informative error messages, code smells... labels May 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug UX user-experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants