From 6026a3eaf235b59671ac2437e0c9a4fa8878b856 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Mon, 17 May 2021 11:37:21 +0200 Subject: Resolve FIXMEs and TODOs Prompted by the article linked below, for each FIXME/TODO I've either referenced an issue in the tracker, or removed it if it's not worth fixing. https://schleiss.io/plotting-source-code-todos-for-open-source-projects --- libcrystfel/src/cell-utils.c | 3 +-- libcrystfel/src/cell.c | 4 ++-- libcrystfel/src/image-cbf.c | 3 +-- libcrystfel/src/image-hdf5.c | 2 +- libcrystfel/src/image.c | 4 +--- libcrystfel/src/indexers/mosflm.c | 28 +--------------------------- libcrystfel/src/indexers/taketwo.c | 2 -- libcrystfel/src/integration.c | 2 +- libcrystfel/src/predict-refine.c | 2 +- libcrystfel/src/reflist-utils.c | 2 +- libcrystfel/src/stream.c | 1 - scripts/plot-cc-and-scale.R | 2 -- src/crystfel_gui.c | 2 +- src/crystfelimageview.c | 2 +- src/geoptimiser.c | 6 +++--- 15 files changed, 15 insertions(+), 50 deletions(-) diff --git a/libcrystfel/src/cell-utils.c b/libcrystfel/src/cell-utils.c index 31ba9d35..30744bd4 100644 --- a/libcrystfel/src/cell-utils.c +++ b/libcrystfel/src/cell-utils.c @@ -1332,8 +1332,7 @@ double lowest_reflection(UnitCell *cell) signed int h, k, l; double lowres = INFINITY; - /* FIXME: Inelegant and nasty. Anyone want to work out - * all the possible cases? */ + /* Boring brute-force approach */ for ( h=0; h<4; h++ ) { for ( k=0; k<4; k++ ) { for ( l=0; l<4; l++ ) { diff --git a/libcrystfel/src/cell.c b/libcrystfel/src/cell.c index 4dea2ce1..775567e3 100644 --- a/libcrystfel/src/cell.c +++ b/libcrystfel/src/cell.c @@ -884,7 +884,7 @@ UnitCell *cell_transform_rational(UnitCell *cell, RationalMatrix *m) ncen = determine_centering(m, cell_get_centering(cell)); cell_set_centering(out, ncen); - /* FIXME: Update unique axis, lattice type */ + /* FIXME: Update unique axis, lattice type (GitLab #39) */ cell_set_lattice_type(out, L_TRICLINIC); cell_set_unique_axis(out, '?'); @@ -972,7 +972,7 @@ UnitCell *cell_transform_rational_inverse(UnitCell *cell, RationalMatrix *m) out = cell_transform_gsl_direct(cell, inv); - /* FIXME: Update centering, unique axis, lattice type */ + /* FIXME: Update centering, unique axis, lattice type (GitLab #39) */ gsl_matrix_free(tm); gsl_matrix_free(inv); diff --git a/libcrystfel/src/image-cbf.c b/libcrystfel/src/image-cbf.c index 19b38607..859ef731 100644 --- a/libcrystfel/src/image-cbf.c +++ b/libcrystfel/src/image-cbf.c @@ -635,8 +635,7 @@ int image_cbf_read(struct image *image, int image_cbf_read_header_to_cache(struct image *image, const char *from) { - /* FIXME: Implementation - * https://gitlab.desy.de/thomas.white/crystfel/-/issues/10 */ + /* FIXME: Implementation (GitLab #10) */ ERROR("Reading headers from CBF files is not currently supported.\n"); return 1; } diff --git a/libcrystfel/src/image-hdf5.c b/libcrystfel/src/image-hdf5.c index 2995654d..497ce763 100644 --- a/libcrystfel/src/image-hdf5.c +++ b/libcrystfel/src/image-hdf5.c @@ -685,7 +685,7 @@ int image_hdf5_read_header_to_cache(struct image *image, const char *name) type = H5Dget_type(dh); class = H5Tget_class(type); - /* FIXME: Handle strings as well */ + /* FIXME: Handle strings as well (GitLab #36) */ if ( (class != H5T_FLOAT) && (class != H5T_INTEGER) ) { ERROR("Not a floating point or integer value.\n"); close_hdf5(fh); diff --git a/libcrystfel/src/image.c b/libcrystfel/src/image.c index 10313ca8..7ec79eaf 100644 --- a/libcrystfel/src/image.c +++ b/libcrystfel/src/image.c @@ -462,7 +462,7 @@ static double get_value(struct image *image, const char *from, if ( image_read_header_float(image, from, &val) == 0 ) { return val; } else { - return NAN; /* FIXME: Use out-of-band flag */ + return NAN; /* FIXME: Use out-of-band flag (GitLab #37) */ } } @@ -767,10 +767,8 @@ static void set_image_parameters(struct image *image, image->bw = dtempl->bandwidth; - /* FIXME: Possibly load spectrum from file */ image->spectrum = spectrum_generate_gaussian(image->lambda, image->bw); - } diff --git a/libcrystfel/src/indexers/mosflm.c b/libcrystfel/src/indexers/mosflm.c index 02532103..c56f3ae6 100644 --- a/libcrystfel/src/indexers/mosflm.c +++ b/libcrystfel/src/indexers/mosflm.c @@ -29,31 +29,6 @@ * */ -/* TODO: - * - * Properly read the newmat file (don't use fscanf-- spaces between numers - * are not guaranteed) - * - * "Success" is indicated by existence of NEWMAT file written by mosflm. - * Better to interact with mosflm directly in order to somehow verify success. - * - * Investigate how these keywords affect mosflms behavior: - * - * MOSAICITY - * DISPERSION - * DIVERGENCE - * POLARISATION - * POSTREF BEAM - * POSTREF USEBEAM OFF - * PREREFINE ON - * EXTRA ON - * POSTREF ON - * - * These did not seem to affect the results by my (Rick's) experience, probably - * because they are only used conjunction with image intensity data, but it's - * worth another look at the documentation. - */ - #include #include @@ -575,8 +550,7 @@ static void mosflm_send_next(struct image *image, struct mosflm_data *mosflm) &a, &b, &c, &alpha, &beta, &gamma); cen = cell_get_centering(mosflm->mp->template); /* Old MOSFLM simply ignores CELL and CENTERING subkeywords. - * So this doesn't do any harm. - * TODO: but still better to show WARNING if MOSFLM is old. */ + * So this doesn't do any harm. */ snprintf(tmp, 255, "AUTOINDEX DPS FILE %s IMAGE 1 " "MAXCELL 1000 REFINE " "CELL %.1f %.1f %.1f %.1f %.1f %.1f " diff --git a/libcrystfel/src/indexers/taketwo.c b/libcrystfel/src/indexers/taketwo.c index 4b481e93..8ce33083 100644 --- a/libcrystfel/src/indexers/taketwo.c +++ b/libcrystfel/src/indexers/taketwo.c @@ -248,8 +248,6 @@ struct TakeTwoCell /* Final required step size for refinement of solutions */ #define ANGLE_CONVERGE_SIZE (deg2rad(0.01)) -/* TODO: Multiple lattices */ - /* ------------------------------------------------------------------------ * apologetic function diff --git a/libcrystfel/src/integration.c b/libcrystfel/src/integration.c index 45f5f677..b2b6ccfa 100644 --- a/libcrystfel/src/integration.c +++ b/libcrystfel/src/integration.c @@ -355,7 +355,7 @@ static void fit_gradient_bg(struct intcontext *ic, struct peak_box *bx) } } - /* FIXME: SVD is massive overkill here */ + /* SVD is massive overkill here, but the routine is right there. */ ans = solve_svd(v, bx->bgm, NULL, 0); gsl_vector_free(v); diff --git a/libcrystfel/src/predict-refine.c b/libcrystfel/src/predict-refine.c index b871f43a..9b562fb0 100644 --- a/libcrystfel/src/predict-refine.c +++ b/libcrystfel/src/predict-refine.c @@ -257,7 +257,7 @@ static int pair_peaks(struct image *image, Crystal *cr, get_detector_pos(refl, &fs, &ss); pd = pow(fs - rps[i].peak->fs, 2.0) + pow(ss - rps[i].peak->ss, 2.0); - if ( pd > 10.0 * 10.0 ) continue; /* FIXME Hardcoded distance */ + if ( pd > 10.0 * 10.0 ) continue; /* FIXME Hardcoded distance (GitLab #38) */ rps[n_acc] = rps[i]; rps[n_acc].refl = reflection_new(h, k, l); diff --git a/libcrystfel/src/reflist-utils.c b/libcrystfel/src/reflist-utils.c index c354cb19..2b250eff 100644 --- a/libcrystfel/src/reflist-utils.c +++ b/libcrystfel/src/reflist-utils.c @@ -1077,7 +1077,7 @@ int write_to_mtz(RefList *reflist, cellp[4] = rad2deg(be); cellp[5] = rad2deg(ga); - /* FIXME: Proposed labelling: + /* FIXME: Proposed labelling (GitLab #28): * title = as above * project = basename of folder containing crystfel.project * crystal = name of indexing results run diff --git a/libcrystfel/src/stream.c b/libcrystfel/src/stream.c index 47438614..ba6e13c8 100644 --- a/libcrystfel/src/stream.c +++ b/libcrystfel/src/stream.c @@ -1082,7 +1082,6 @@ struct image *stream_read_chunk(Stream *st, StreamFlags srf) image_set_zero_data(image, st->dtempl_read); image_set_zero_mask(image, st->dtempl_read); } - /* FIXME: Maybe arbitrary spectrum from file (?) */ image->spectrum = spectrum_generate_gaussian(image->lambda, image->bw); return image; diff --git a/scripts/plot-cc-and-scale.R b/scripts/plot-cc-and-scale.R index b67ab058..97c643d3 100644 --- a/scripts/plot-cc-and-scale.R +++ b/scripts/plot-cc-and-scale.R @@ -5,8 +5,6 @@ scale_cc <- read.table("stats.dat", col.names=c("ImageName", "ScaleFactor", "CC")) # scatter plot -# FIXME: sometimes white lines appear on the plot. -# seems to be a bug in smoothScatter smoothScatter(scale_cc$ScaleFactor, scale_cc$CC, nbin=200, xlim=c(0, 2), ylim=c(0, 1), diff --git a/src/crystfel_gui.c b/src/crystfel_gui.c index 28661064..700cc4f1 100644 --- a/src/crystfel_gui.c +++ b/src/crystfel_gui.c @@ -640,7 +640,7 @@ static void add_button(GtkWidget *vbox, const char *label, const char *imagen, static void add_task_buttons(GtkWidget *vbox, struct crystfelproject *proj) { - /* FIXME: All these icons are placeholders */ + /* FIXME: All these icons are placeholders (GitLab #9) */ add_button(vbox, "Load data", "folder-pictures", G_CALLBACK(import_sig), proj); add_button(vbox, "Peak detection", "edit-find", diff --git a/src/crystfelimageview.c b/src/crystfelimageview.c index 8302ae44..12ef1dee 100644 --- a/src/crystfelimageview.c +++ b/src/crystfelimageview.c @@ -239,7 +239,7 @@ static void draw_pixel_values(cairo_t *cr, PangoFontDescription *fontdesc; double w, h; - /* FIXME: This is wrong for slanty pixels */ + /* FIXME: This is wrong for slanty pixels (GitLab #17) */ min_fs = clamp(imin_fs, 0, p.w-1); min_ss = clamp(imin_ss, 0, p.h-1); max_fs = clamp(imax_fs, 0, p.w-1); diff --git a/src/geoptimiser.c b/src/geoptimiser.c index 24a159de..f28e9551 100644 --- a/src/geoptimiser.c +++ b/src/geoptimiser.c @@ -1074,7 +1074,7 @@ static DataTemplate *correct_rotation_and_stretch(struct rg_collection *connecte struct geoptimiser_params *gparams) { #if 0 - /* FIXME ! */ + /* FIXME (GitLab #29) */ int di, ip; STATUS("Applying rotation and stretch corrections.\n"); @@ -1392,7 +1392,7 @@ static void correct_shift(struct rg_collection *connected, double clen_to_use) { #if 0 - /* FIXME ! */ + /* FIXME (GitLab #29) */ int di; int ip; @@ -1585,7 +1585,7 @@ static double compute_rotation_and_stretch(struct rg_collection *connected, num_pix_first_p = first_p->w * first_p->h; - /* FIXME: minrad here is not universal */ + /* FIXME: minrad here is not universal (GitLab #29) */ min_dist = dist_coeff_for_rot_str * sqrt(num_pix_first_p * connected->rigid_groups[di]->n_panels); -- cgit v1.2.3