Raise more informative exception in Table value formatting if type and format mismatch#3055
Raise more informative exception in Table value formatting if type and format mismatch#3055cherti wants to merge 7 commits intoastropy:masterfrom
Conversation
…ion in Exception-message
…fficient and more efficient) and test all existing entries for np.object-columns to make sure it doesn't crash at least with the existing ones
astropy/table/column.py
Outdated
There was a problem hiding this comment.
Here you can just do:
raise
(no need for the TypeError...) - and this way you preserve the traceback
|
@cherti - I renamed your PR with a more informative title -- feel free to change if it doesn't quite capture what you want. |
|
@cherti There's test errors on travis-ci: Are you able to reproduce and debug those locally by running this? Also there's merge conflicts ... could you please rebase this pull request against master? |
|
Actually I currently have a problem with even a freshly cloned astropy being unable to run all the tests without throwing an Exception(*). Unfortunately I haven't had the time to debug this one, therefore I cannot test my stuff. I'll do the rebase as soon as I figured out the weird issue with the testing. If there wasn't that, I'd have this Pull Request finished for merge, however, I hadn't had enough time up to now to fully debug the error with the tests as it seems a little weird to me and I haven't figured out completely what's going wrong. I think there is no point in doing the rebase now if I cannot run the tests, is there? Sorry for the delay. *) |
There was a problem hiding this comment.
I'm not exactly clear what this line is supposed to do--is this just checking that the dtype is object? As is this will always return False--type() returns a class, and you're just comparing that class to a bunch of strings.
Column implementation in master. This also made a few changes to methods like __array_finalize__ and __setstate__ to not go through the setters for name, unit, and format--these are copying/recreating Columns from existing objects that should presumably already have valid values for those attributes. This also addresses my comment on the original PR here: astropy#3055 (comment) Finally, in order for this to pass testing several existing tests needed to be updated, now that we no longer allow a Column with an invalid format to be created in the first place (a change I support).
Column implementation in master. This also made a few changes to methods like __array_finalize__ and __setstate__ to not go through the setters for name, unit, and format--these are copying/recreating Columns from existing objects that should presumably already have valid values for those attributes. This also addresses my comment on the original PR here: astropy#3055 (comment) Finally, in order for this to pass testing several existing tests needed to be updated, now that we no longer allow a Column with an invalid format to be created in the first place (a change I support).
|
@embray Does that clarify what I intended in that line? |
|
@cherti Somewhat...I think...it would be helpful if you added a regression test that exemplifies exactly the situation. From the sound of it though I think that additional check can be removed pretty much entirely. |
Column implementation in master. This also made a few changes to methods like __array_finalize__ and __setstate__ to not go through the setters for name, unit, and format--these are copying/recreating Columns from existing objects that should presumably already have valid values for those attributes. This also addresses my comment on the original PR here: astropy#3055 (comment) Finally, in order for this to pass testing several existing tests needed to be updated, now that we no longer allow a Column with an invalid format to be created in the first place (a change I support).
Column implementation in master. This also made a few changes to methods like __array_finalize__ and __setstate__ to not go through the setters for name, unit, and format--these are copying/recreating Columns from existing objects that should presumably already have valid values for those attributes. This also addresses my comment on the original PR here: astropy#3055 (comment) Finally, in order for this to pass testing several existing tests needed to be updated, now that we no longer allow a Column with an invalid format to be created in the first place (a change I support).
Column implementation in master. This also made a few changes to methods like __array_finalize__ and __setstate__ to not go through the setters for name, unit, and format--these are copying/recreating Columns from existing objects that should presumably already have valid values for those attributes. This also addresses my comment on the original PR here: astropy#3055 (comment) Finally, in order for this to pass testing several existing tests needed to be updated, now that we no longer allow a Column with an invalid format to be created in the first place (a change I support).
Implementing Issue #2868:
Implementing raising TypeError upon nonworking format string instead of value error, handling it above the point where it was originally raised to include information about the exact point of failure.
Additionally getter and setter were implemented to raise the error as soon as it can be determined to not work (aka when setting it).
Format is checked for all existing entries in case of np.object-ish and for one entry for other types as this should be sufficient and more efficient for large tables.