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

BUG: fix error messages in RAMSESDataset validation #3801

Merged
merged 1 commit into from
Feb 14, 2022

Conversation

neutrinoceros
Copy link
Member

@neutrinoceros neutrinoceros commented Feb 10, 2022

PR Summary

This adresses the surface problem reported in #3800, i.e., I'm demystifying errors raised by RAMSESDataset at validation time

Example 1

from yt.frontends.ramses.api import RAMSESDataset

RAMSESDataset("not_a_file")

On main

ValueError: ('Invalid filename found when building a RAMSESDataset object: %s', 'not_a_file')

This branch

FileNotFoundError: No such file or directory '/Users/robcleme/dev/yt-project/yt/not_a_file'

Example 2

import os
from yt.frontends.ramses.api import RAMSESDataset

RAMSESDataset(os.path.expanduser("~"))

On main

ValueError: ('Invalid filename found when building a RAMSESDataset object: %s', '/Users/robcleme')

This branch

ValueError: Could not determine output directory from '/Users/robcleme'
Expected a directory name of form '(output|group)_(\\d{5})'

@neutrinoceros neutrinoceros added bug code frontends Things related to specific frontends UX user-experience labels Feb 10, 2022
@neutrinoceros neutrinoceros added this to the 4.0.3 milestone Feb 10, 2022
matthewturk
matthewturk previously approved these changes Feb 10, 2022
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.

Good improvement.

@neutrinoceros
Copy link
Member Author

@matthewturk sorry I just forced pushed again because I realised I made a mistake that could break yt.load(None)

@neutrinoceros neutrinoceros force-pushed the hotfix_errors_3800 branch 5 times, most recently from e5dca76 to ed7c7a3 Compare February 11, 2022 07:42
@neutrinoceros
Copy link
Member Author

This took a surprising amount of iterations for me to understand enough of what the frontend actually expected so I can provide more useful error messages. So I think the problem Andrew actually had was that his data isn't stored in a dir that's named output_xxxxx or group_xxxxxx. I've now made this expectation much more apparent in the error message :

ValueError: Could not determine output directory from '/Users/robcleme'
Expected a directory name of form '(output|group)_(\\d{5})'

I'll wait on his reply to see if he's indeed able to load his data after rearranging the files, but I expect that this is now sufficient to close the issue.

@neutrinoceros neutrinoceros linked an issue Feb 11, 2022 that may be closed by this pull request
Copy link
Member

@cphyc cphyc left a comment

Choose a reason for hiding this comment

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

LGTM

@cphyc cphyc merged commit 5f1f66a into yt-project:main Feb 14, 2022
@neutrinoceros neutrinoceros deleted the hotfix_errors_3800 branch February 14, 2022 11:00
@cphyc
Copy link
Member

cphyc commented Feb 14, 2022

Ooops, I should have waited a bit more before merging, shouldn't I?

@neutrinoceros
Copy link
Member Author

I don't think so, why ?

@neutrinoceros
Copy link
Member Author

(If it turns out that the issue Andrew had was more complex I can always reopen the ticket)

neutrinoceros added a commit that referenced this pull request Feb 14, 2022
…1-on-yt-4.0.x

Backport PR #3801 on branch yt-4.0.x (BUG: fix error messages in RAMSESDataset validation)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug code frontends Things related to specific frontends UX user-experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: RAMSESDataset fails to load
3 participants