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.

Sign up for GitHub

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

Jump to bottom

repr for VOTable is now clearer #14702

Merged
merged 1 commit into from
Jun 2, 2023
Merged

repr for VOTable is now clearer #14702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions astropy/io/votable/tests/vo_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,9 @@
# Resource
assert repr(self.votable.resources) == "[</>]"

# Table
assert repr(self.table).startswith("<VOTable")

Check warning on line 600 in astropy/io/votable/tests/vo_test.py

View check run for this annotation

Codecov / codecov/patch

astropy/io/votable/tests/vo_test.py#L600 

Added line #L600 was not covered by tests


class TestThroughTableData(TestParse):
def setup_class(self):
Expand Down
5 changes: 4 additions & 1 deletion astropy/io/votable/tree.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2420,7 +2420,10 @@
warn_unknown_attrs("TABLE", extra.keys(), config, pos)

def __repr__(self):
return repr(self.to_table())
s = repr(self.to_table())
if s.startswith("<Table"):
s = "<VO" + s[1:]
Comment on lines +2423 to +2425
Copy link
Member

Choose a reason for hiding this comment

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

Looking at this again, now I wonder why the conditional?

Copy link
Member Author

Choose a reason for hiding this comment

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

To guard against something else breaking unexpectedly because this is old code and I don't understand the whole stack in this subpackage. So I chose to be very explicit on when to overwrite the repr. Did this if break stuff for you?

Copy link
Member

Choose a reason for hiding this comment

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

No break, I'm just about to do some more similar things in pyvo and spotted this as something odd

Copy link
Member Author

Choose a reason for hiding this comment

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

If you are sure you don't need the if, you can remove it. I am just not 100% sure and don't have time to investigate. Thanks!

return s

Check warning on line 2426 in astropy/io/votable/tree.py

View check run for this annotation

Codecov / codecov/patch

astropy/io/votable/tree.py#L2423-L2426 

Added lines #L2423 - L2426 were not covered by tests

def __bytes__(self):
return bytes(self.to_table())
Expand Down
1 change: 1 addition & 0 deletions docs/changes/io.votable/14702.feature.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Output of ``repr`` for VOTable instance now clearly shows it is a VOTable and not generic astropy Table.