Add information line on SVG export to give information on image size #1147
Conversation
There was a problem hiding this comment.
I made code comments - but overall, I think the look and feel of this could be improved. The three lines, without spacing, don't look that great. What about expanding the width of the dialog? Maybe bounce this around to other people and see if they have suggestions.
nion/swift/ExportDialog.py
Outdated
| calibrated_image_size = (self.__display_item.dimensional_shape[0] * self.__display_item.data_item.dimensional_calibrations[0].scale, | ||
| self.__display_item.dimensional_shape[1] * self.__display_item.data_item.dimensional_calibrations[1].scale) | ||
| data_information += ("(" + str(calibrated_image_size[0]) + " " + self.__display_item.data_item.dimensional_calibrations[0].units + "," + | ||
| str(calibrated_image_size[1]) + " " + self.__display_item.data_item.dimensional_calibrations[1].units + ")") | ||
| else: | ||
| data_information += "Line Plot Size " + str(self.__display_item.dimensional_shape[0]) | ||
| calibrated_line_plot_size = self.__display_item.dimensional_shape[0] * self.__display_item.data_item.dimensional_calibrations[0].scale | ||
| data_information += ("(" + str(calibrated_line_plot_size) + " " + self.__display_item.data_item.dimensional_calibrations[0].units + ")") |
There was a problem hiding this comment.
To avoid having to look into the display_item.data_item, which is None for composite displays, you can use display_item.displayed_dimensional_calibrations which will give the dimensional calibrations for images, line plots, and composites.
nion/swift/ExportDialog.py
Outdated
| u = Declarative.DeclarativeUI() | ||
| self._float_to_string_converter = Converter.FloatToStringConverter() | ||
| self.ui_view = u.create_column( | ||
| self.ui_view = u.create_column(u.create_row( | ||
| u.create_label(text="@binding(model.image_info)", word_wrap=True), spacing=12), |
There was a problem hiding this comment.
Is there a reason you have a row to contain a single object (the label)?
There was a problem hiding this comment.
First note is that this PR doesn't rebase cleanly, so you might want to consider closing and opening a new PR with just the one file. Alternatively you can rebase it yourself and force push to your PR branch.
Next, I think you need to have a uniform way of displaying calibrations with the correct precision. The inspector has a way to do this based on the data dimensions.
Review the code in the CalibratedSizeFloatToStringConverter class in Inspector.py, particularly the convert_calibrated_value_to_str method of that class. I think you can use that class directly.
Without that fix, I get results like this (from exporting the Ronchigram simulator image):
lisham2000
force-pushed
the
SVGInfoLabel
branch
from
5d93fa6
to
1f40e95
Compare
There was a problem hiding this comment.
Overall this looks very close (see my comments about using displayed_dimensional_calibrations from today; and my comments about using CalibratedSizeFloatToStringConverter from last week), but also needs a clean commit. Easiest way from your branch:
git reset --soft master
git commit -m "Add data info line to SVG export dialog."
git show # to ensure it's as expected
git push --force
nion/swift/ExportDialog.py
Outdated
| assert self.__display_item.data_item and self.__display_item.dimensional_shape | ||
| calibrated_image_size = (self.__display_item.dimensional_shape[-2] * self.__display_item.data_item.dimensional_calibrations[-2].scale, | ||
| self.__display_item.dimensional_shape[-1] * self.__display_item.data_item.dimensional_calibrations[-1].scale) | ||
| data_information += ("(" + str(calibrated_image_size[-2]) + " " + self.__display_item.data_item.dimensional_calibrations[-2].units + "," + | ||
| str(calibrated_image_size[-1]) + " " + self.__display_item.data_item.dimensional_calibrations[-1].units + ")") | ||
| else: |
There was a problem hiding this comment.
Can you change these lines to use display_item.displayed_dimensional_calibrations like the other clause?
lisham2000
force-pushed
the
SVGInfoLabel
branch
from
18dd032
to
41d6dec
Compare
I have run these git commands and it has put it to one commit but it now says it has changes 22 files just checking this is right? |
|
On further review this doesnt look right at all and it is no longer building. With the state of this PR to get it into one clean commit i am closing this PR and it has been replaced by #1192 |
No description provided.