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.

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

Only show format if not null #1981

Merged
merged 7 commits into from
Jan 24, 2022
Merged

Only show format if not null #1981

merged 7 commits into from
Jan 24, 2022

Conversation

graeme-winter
Copy link
Contributor

i.e. not Format or FormatMultiImage - used to just check was not Format.
Avoids confusion in looking at dials.show output.

Fixes #1980

i.e. not Format or FormatMultiImage - used to just check was not Format.
Avoids confusion in looking at dials.show output.

Fixes #1980
@codecov
Copy link

codecov bot commented Jan 18, 2022

Codecov Report

Merging #1981 (7603aae) into main (7df0b0d) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 7603aae differs from pull request most recent head fdaeea6. Consider uploading reports for the commit fdaeea6 to get more accurate results

@@           Coverage Diff           @@
##             main    #1981   +/-   ##
=======================================
  Coverage   67.64%   67.64%           
=======================================
  Files         629      629           
  Lines       71400    71400           
  Branches    10471    10471           
=======================================
  Hits        48295    48295           
  Misses      21125    21125           
  Partials     1980     1980           

@@ -250,7 +250,7 @@ def show_experiments(experiments, show_scan_varying=False):
for i_expt, expt in enumerate(experiments):
text.append("Experiment %i:" % i_expt)
format_class = expt.imageset.get_format_class()
if format_class.__name__ != "Format":
if format_class.__name__ not in ["Format", "FormatMultiImage"]:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if format_class.__name__ not in ["Format", "FormatMultiImage"]:
if not format_class.abstract():

Would this be more robust?

Copy link
Member

Choose a reason for hiding this comment

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

Possibly, but if not:

Suggested change
if format_class.__name__ not in ["Format", "FormatMultiImage"]:
if format_class.__name__ not in {"Format", "FormatMultiImage"}:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

abstract() not always defined 🤔

and it turns out FormatMultiImage is not abstract, either

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done subsequent suggestion instead.

Copy link
Member

Choose a reason for hiding this comment

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

oh ffs it's is_abstract

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh ffs it's is_abstract

Probably yeh, will try that later

@ndevenish
Copy link
Member

This test is now failing because of cctbx/dxtbx#463

@ndevenish ndevenish merged commit 20f7546 into main Jan 24, 2022
@ndevenish ndevenish deleted the show-null-format branch January 24, 2022 10:51
ndevenish added a commit that referenced this pull request Jan 24, 2022
If the images are not available or running in CHECK_FORMAT mode, then
the concrete type is not available and only one of the abstract root formats
will be resolved. In those cases, it's not useful to print out the format type.

Co-authored-by: Nicholas Devenish <ndevenish@gmail.com>
DiamondLightSource-build-server added a commit that referenced this pull request Jan 24, 2022
Bugfixes
--------

- Correct reporting of phi angle in scan-varying model plots. (#1929)
- ``dials.sequence_to_stills``: correct error in the crystal model for scans that do not start from image 1. (#1933)
- `dials.show`: only show format class if meaningful (i.e. not Format or FormatMultiImage) (#1981)

Improved Documentation
----------------------

- Update some Cosym and resolution-related PHIL descriptions. (#1969)
DiamondLightSource-build-server added a commit that referenced this pull request Jan 25, 2022
Bugfixes
--------

- Correct reporting of phi angle in scan-varying model plots. (#1929)
- ``dials.sequence_to_stills``: correct error in the crystal model for scans that do not start from image 1. (#1933)
- `dials.show`: only show format class if meaningful (i.e. not Format or FormatMultiImage) (#1981)

Improved Documentation
----------------------

- Update some Cosym and resolution-related PHIL descriptions. (#1969)
DiamondLightSource-build-server added a commit that referenced this pull request Jan 25, 2022
Features
--------

- It is now possible to explicitly bootstrap all supported python versions. (`#XXX <https://github.com/dials/dials/issues/XXX>`_)

Bugfixes
--------

- Correct reporting of phi angle in scan-varying model plots. (#1929)
- ``dials.sequence_to_stills``: correct error in the crystal model for scans that do not start from image 1. (#1933)
- `dials.show`: only show format class if meaningful (i.e. not Format or FormatMultiImage) (#1981)

Improved Documentation
----------------------

- Update some Cosym and resolution-related PHIL descriptions. (#1969)
DiamondLightSource-build-server added a commit that referenced this pull request Jan 25, 2022
Features
--------

- It is now possible to explicitly bootstrap all supported python versions. (#1988)

Bugfixes
--------

- Correct reporting of phi angle in scan-varying model plots. (#1929)
- ``dials.sequence_to_stills``: correct error in the crystal model for scans that do not start from image 1. (#1933)
- `dials.show`: only show format class if meaningful (i.e. not Format or FormatMultiImage) (#1981)

Improved Documentation
----------------------

- Update some Cosym and resolution-related PHIL descriptions. (#1969)
DiamondLightSource-build-server added a commit that referenced this pull request Jan 25, 2022
Features
--------

- It is now possible to explicitly bootstrap all supported python versions. (#1988)

Bugfixes
--------

- ``dials.refine``: Fix crash from recording of parameter correlations in ``history.json``. (#1923)
- Correct reporting of phi angle in scan-varying model plots. (#1929)
- ``dials.sequence_to_stills``: correct error in the crystal model for scans that do not start from image 1. (#1933)
- `dials.show`: only show format class if meaningful (i.e. not Format or FormatMultiImage) (#1981)

Improved Documentation
----------------------

- Update some Cosym and resolution-related PHIL descriptions. (#1969)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dials.show shows the wrong format class
4 participants