Skip to content

Include information on output-only in unknown formats error message. - #11831

Merged
pllim merged 1 commit into
astropy:mainfrom
mhvk:unit-format-list-improvement
Jun 10, 2021
Merged

Include information on output-only in unknown formats error message.#11831
pllim merged 1 commit into
astropy:mainfrom
mhvk:unit-format-list-improvement

Conversation

@mhvk

@mhvk mhvk commented Jun 9, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

@olebole - it is not too simple to change the error message specific to input or output, as it happens inside units.format, but how about this improved message?

ValueError: Unknown format 'foo'.  Valid formatter names are: [['generic', 'unscaled', 'cds', 'console*', 'fits', 'latex*', 'latex_inline*', 'ogip', 'unicode*', 'vounit']] (where those marked with a * are output-only)

EDIT: final change to

ValueError: Unknown format 'foo'.  Valid formatter names are: ['generic', 'unscaled', 'cds', 'fits', 'ogip', 'vounit'] for input and output, and ['console', 'latex', 'latex_inline', 'unicode'] for output only.

@mhvk mhvk added this to the v5.0 milestone Jun 9, 2021
This was referenced Jun 9, 2021
Merged
Merged
@mhvk
mhvk force-pushed the unit-format-list-improvement branch from b341958 to 2ab90c2 Compare June 9, 2021 23:53
@mhvk
mhvk force-pushed the unit-format-list-improvement branch from 2ab90c2 to f4d0298 Compare June 10, 2021 00:38
@mhvk

mhvk commented Jun 10, 2021

Copy link
Copy Markdown
Contributor Author

p.s. Changed the error message again, now I think much better.

@olebole

olebole commented Jun 10, 2021

Copy link
Copy Markdown
Member

Yes, I think it is a good compromise. Thank you.

@mhvk

mhvk commented Jun 10, 2021

Copy link
Copy Markdown
Contributor Author

Great! @pllim - sorry to bother, but with @olebole's OK, would you be able to approve & merge?

pllim
pllim approved these changes Jun 10, 2021
View reviewed changes

@pllim pllim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No problem!

@pllim
pllim merged commit 9639593 into astropy:main Jun 10, 2021
@pllim

pllim commented Jun 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

For future reference, this is a direct follow-up of #11829 , correct?

xref #11828

@pllim pllim added the Affects-dev PRs and issues that do not impact an existing Astropy release label Jun 10, 2021
@mhvk

mhvk commented Jun 10, 2021

Copy link
Copy Markdown
Contributor Author

Indeed, to also clarify the error message for the case of an unknown format.

@mhvk
mhvk deleted the unit-format-list-improvement branch June 10, 2021 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pllim pllim pllim approved these changes

Assignees

No one assigned

Labels

Affects-dev PRs and issues that do not impact an existing Astropy release no-changelog-entry-needed Refactoring units

Projects

None yet

Milestone

v5.0

Development

Successfully merging this pull request may close these issues.

3 participants

@mhvk @olebole @pllim