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

Code cleanup for dangling file pointer investigation #316

Merged
merged 3 commits into from
Feb 23, 2021

Conversation

Anthchirp
Copy link
Member

Investigation into dials/dials#1603 found some superfluous programming motifs that made it unnecessarily difficult to figure out what is going on.

@codecov
Copy link

codecov bot commented Feb 23, 2021

Codecov Report

Merging #316 (853a454) into master (d1f9583) will increase coverage by 0.03%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##           master     #316      +/-   ##
==========================================
+ Coverage   46.88%   46.92%   +0.03%     
==========================================
  Files         231      231              
  Lines       19166    19129      -37     
  Branches     2758     2745      -13     
==========================================
- Hits         8986     8976      -10     
+ Misses       9683     9658      -25     
+ Partials      497      495       -2     

Copy link
Collaborator

@graeme-winter graeme-winter left a comment

Choose a reason for hiding this comment

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

Nice clean up 👍

Tests pass, code makes sense, expect there is more of this nonsense to be found but this still helps thank you

@@ -1502,7 +1501,7 @@ def from_dict(obj, check_format=True, directory=None):
def from_json(string, check_format=True, directory=None):
"""Decode a datablock from JSON string."""
return DataBlockFactory.from_dict(
json.loads(string, object_hook=_decode_dict),
json.loads(string),
Copy link
Collaborator

Choose a reason for hiding this comment

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

was the object_hook stuff really useless?

Copy link
Member Author

Choose a reason for hiding this comment

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

on Python 3 it was a no-op

@@ -1,14 +1,9 @@
from __future__ import absolute_import, division, print_function
Copy link
Collaborator

Choose a reason for hiding this comment

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

🙂


from dxtbx.model.crystal import CrystalFactory
from dxtbx.serialize.imageset import imageset_from_dict


def _decode_list(data):
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is only useless now we are in the future 👍

@Anthchirp Anthchirp merged commit c9530e8 into master Feb 23, 2021
@Anthchirp Anthchirp deleted the dangling-filepointer branch February 23, 2021 11:15
Anthchirp added a commit that referenced this pull request Apr 22, 2021
Anthchirp added a commit that referenced this pull request Apr 23, 2021
Remove deprecated classes (previously deprecated in #302)
  dxtbx.datablock.*Diff
  dxtbx.model.experiment_list.SequenceDiff

Remove deprecated function (previously deprecated in #316)
  dxtbx.serialize.load.imageset_from_string
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.

3 participants