Skip to content

Made Latex/AASTex tables use unit attribute of Column for output#3064

Merged
taldcroft merged 3 commits into
astropy:masterfrom
mdmueller:table-units-latex
Nov 16, 2014
Merged

Made Latex/AASTex tables use unit attribute of Column for output#3064
taldcroft merged 3 commits into
astropy:masterfrom
mdmueller:table-units-latex

Conversation

@mdmueller

Copy link
Copy Markdown
Contributor

This is an implementation of the suggestion in #2870.

@hamogu

hamogu commented Nov 12, 2014

Copy link
Copy Markdown
Member

Looks good to me.
CC @taldcroft

@taldcroft

Copy link
Copy Markdown
Member

@mdmueller - just got back from vacation, will have a look this week.

@mdmueller

Copy link
Copy Markdown
Contributor Author

Sounds good!

Comment thread astropy/io/ascii/latex.py Outdated

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.

I suggest .unit.to_string(format='latex_inline'). Particualrly useful if the unit is Angstroem or \mu that can actually be written with the real symbol in Latex.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This seems to change the behavior of the tests, e.g. "s" becomes "$\mathrm{s}$". Should I just change the expected test output then?

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.

Yes, please change the expected output of the tests.
LaTeX can encode symbols and formulas that plain ASCII cannot (e.g. the
A with a circle on the top that means Angstroem, or proper use of
exponents, try e.g. a unit like u.km / u.s**2). When we use a LaTeX
writer, we should make full use of astropy's ability to write the units
in LaTeX form with all the bells and whistles that normal ASCII output
does not have.

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.

I agree with @hamogu to use the latex_inline format and change the tests accordingly.

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.

One more thing I just realized: What happens is no unit is set?
In that case I would prefer not to see an empty line. What do you think?

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.

Agree, if there are no columns with a defined unit then there should not be an empty line.

@hamogu

hamogu commented Nov 12, 2014

Copy link
Copy Markdown
Member

I run one of the output through LaTeX today and it loogs good apart from the comments I left above.
Note that LaTeX actually fails with the example we test (it always did, that is not related to this PR), because the value of the PERROR column is No_error and LaTeX thinks that the _ is a formula and should be enclosed in no_error.
That's nothinkg to worry about. If the table has strings that have a special meaning in LaTeX then the user has to deal with that. I just wanted to note that here in case somebody else wants to try that out.

Comment thread astropy/io/ascii/latex.py Outdated

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.

Maybe you can tidy up the logic a bit with something like:

units = self.latex.get('units', {})
units.update((x.name, x.unit.to_string(format='latex_inline')) for x in self.cols if x.unit)
if units:
    lines.append(self.splitter.join([units.get(x.name, ' ') for x in self.cols)])

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.

Oops, just realized I have the precedence wrong here, so you need to flip things.

@mdmueller

Copy link
Copy Markdown
Contributor Author

I just added a new commit--does this look okay?

@taldcroft

Copy link
Copy Markdown
Member

Looks OK but you used a dict comprehension which doesn't exist in Python 2.6. Have to use the dict((key, val) for ..) idiom.

@mdmueller

Copy link
Copy Markdown
Contributor Author

Ok, updated--waiting on Travis.

taldcroft added a commit that referenced this pull request Nov 16, 2014
Made Latex/AASTex tables use unit attribute of Column for output
@taldcroft taldcroft merged commit 58f6518 into astropy:master Nov 16, 2014
@taldcroft taldcroft added this to the v1.0.0 milestone Nov 16, 2014
taldcroft added a commit that referenced this pull request Nov 16, 2014
@mdmueller mdmueller deleted the table-units-latex branch November 16, 2014 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants