Skip to content
Snippets Groups Projects

Draft: fix: CCV markdown tablefmt from pipe to grid for a better rst conversion and pdf display

Closed Draft: fix: CCV markdown tablefmt from pipe to grid for a better rst conversion and pdf display
fix/ccv_markdown_tablefmt into master
Closed Karim Ahmed requested to merge fix/ccv_markdown_tablefmt into master

Description

After we started using textlive/2022, the markdown tables for the retrieved constants are not properly converted in RST and Latex.

This MR fixes the issue by introducing an environment variable NB_CONVERT_RUNNING and a display_latex_retrieved_constants function to be able to display_retrieved_constants in either markdown or latex based on the existence of NB_CONVERT_RUNNING

Changes:

  • NB_CONVERT_RUNNING environment variable when notebooks are executed using xfel-calibrate
  • display_retrieved_constants() method deciding in which format the retrieved constants table should be generated.
  • display_latex_retrieved_constants() method to display a table of retrieved constants in latex.
  • Update detector notebooks':
  • Jungfrau
  • epix100
  • pnCCD
  • AGIPD
  • [ ] LPDs
  • Gotthard2

How Has This Been Tested?

  • Automated tests.

Relevant Documents (optional)

This is a screenshot for the current issue: Screenshot_from_2024-07-10_15-20-44

MASTER state

This is how the table looks like in RST files:

+---------+--------------+--------------+--------------+--------------+
| Modules | Noise        | Offset       | BadPixelsFF  | B            |
|         |              |              |              | adPixelsDark |
+=========+==============+==============+==============+==============+
| AGIPD00 | `2021-07-30  | `2021-07-30  | `2021-07-29  | `2021-07-30  |
|         | 21:23        | 21:23        | 17:44        | 21:23        |
|         | <https://in. | <https://in. | <https://in. | <https://in. |
|         | xfel.eu/cali | xfel.eu/cali | xfel.eu/cali | xfel.eu/cali |
|         | bration/cali | bration/cali | bration/cali | bration/cali |
|         | bration_cons | bration_cons | bration_cons | bration_cons |
|         | tant_version | tant_version | tant_version | tant_version |
|         | s//84948>`__ | s//84932>`__ | s//93997>`__ | s//84959>`__ |
+---------+--------------+--------------+--------------+--------------+
| AGIPD01 | `2021-07-30  | `2021-07-30  | `2021-07-29  | `2021-07-30  |
|         | 21:23        | 21:23        | 17:44        | 21:23        |
|         | <https://in. | <https://in. | <https://in. | <https://in. |
|         | xfel.eu/cali | xfel.eu/cali | xfel.eu/cali | xfel.eu/cali |
|         | bration/cali | bration/cali | bration/cali | bration/cali |
|         | bration_cons | bration_cons | bration_cons | bration_cons |
|         | tant_version | tant_version | tant_version | tant_version |
|         | s//84942>`__ | s//84930>`__ | s//93977>`__ | s//84954>`__ |
+---------+--------------+--------------+--------------+--------------+
| AGIPD02 | `2021-07-30  | `2021-07-30  | `2021-07-29  | `2021-07-30  |
|         | 21:23 <      | 21:23 <      | 17:44        | 21:23 <      |
|         | https://in.x | https://in.x | <https://in. | https://in.x |
|         | fel.eu/calib | fel.eu/calib | xfel.eu/cali | fel.eu/calib |
|         | ration/calib | ration/calib | bration/cali | ration/calib |
|         | ration_const | ration_const | bration_cons | ration_const |
|         | ant_versions | ant_versions | tant_version | ant_versions |
|         | //202755>`__ | //202750>`__ | s//93983>`__ | //202760>`__ |
+---------+--------------+--------------+--------------+--------------+
| AGIPD03 | `2021-07-30  | `2021-07-30  | `2021-07-29  | `2021-07-30  |
|         | 21:23 <      | 21:23 <      | 17:44        | 21:23 <      |
|         | https://in.x | https://in.x | <https://in. | https://in.x |
|         | fel.eu/calib | fel.eu/calib | xfel.eu/cali | fel.eu/calib |
|         | ration/calib | ration/calib | bration/cali | ration/calib |
|         | ration_const | ration_const | bration_cons | ration_const |
|         | ant_versions | ant_versions | tant_version | ant_versions |
|         | //202748>`__ | //202747>`__ | s//93979>`__ | //202751>`__ |
+---------+--------------+--------------+--------------+--------------+
| AGIPD04 | `2021-07-30  | `2021-07-30  | `2021-07-29  | `2021-07-30  |
|         | 21:23 <      | 21:23 <      | 17:44        | 21:23 <      |
|         | https://in.x | https://in.x | <https://in. | https://in.x |
|         | fel.eu/calib | fel.eu/calib | xfel.eu/cali | fel.eu/calib |
|         | ration/calib | ration/calib | bration/cali | ration/calib |
|         | ration_const | ration_const | bration_cons | ration_const |
|         | ant_versions | ant_versions | tant_version | ant_versions |
|         | //202762>`__ | //202756>`__ | s//93975>`__ | //202764>`__ |
+---------+--------------+--------------+--------------+--------------+

Current Version

Screenshot_from_2024-07-26_08-01-01

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

Reviewers

@kluyvert @schmidtp

Edited by Karim Ahmed

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Karim Ahmed assigned to @ahmedk

    assigned to @ahmedk

    • Thomas Kluyver
      Thomas Kluyver @kluyvert ·
      Owner

      Thanks! I assume they still look right when displayed in the notebook as well?

      LGTM

    • Karim Ahmed
      Karim Ahmed @ahmedk ·
      Author Owner

      Unfortunately, it is not the case still. I am still looking for another solution. As this looks bad in the notebook.

    • Philipp Schmidt
      Philipp Schmidt @schmidtp ·
      Owner

      The first screenshot is with this MR? The left-most column still seems wrong... it's not math mode, but somehow also not regular text.

    • Karim Ahmed
      Karim Ahmed @ahmedk ·
      Author Owner

      yes, it looks funny.

      This is how it looks in RST.

      +---+----------------+----------------+----------------+----------------+
| M | Noise          | ThresholdsDark | BadPixelsDark  | BadPixelsPC    |
| o |                |                |                |                |
| d |                |                |                |                |
| u |                |                |                |                |
| l |                |                |                |                |
| e |                |                |                |                |
| s |                |                |                |                |
+===+================+================+================+================+
| A | `2021-08-05    | `2021-08-05    | `2021-08-05    | `2021-08-09    |
| G | 16:24 <h       | 16:24 <h       | 16:24 <h       | 14:45 <h       |
| I | ttps://in.xfel | ttps://in.xfel | ttps://in.xfel | ttps://in.xfel |
| P | .eu/calibratio | .eu/calibratio | .eu/calibratio | .eu/calibratio |
| D | n/calibration_ | n/calibration_ | n/calibration_ | n/calibration_ |
| 0 | constant_versi | constant_versi | constant_versi | constant_versi |
| 0 | ons//87560>`__ | ons//87577>`__ | ons//87568>`__ | ons//92116>`__ |
+---+----------------+----------------+----------------+----------------+
| A | `2021-08-05    | `2021-08-05    | `2021-08-05    | `2021-08-09    |
| G | 16:24 <h       | 16:24 <h       | 16:24 <h       | 14:45 <h       |
| I | ttps://in.xfel | ttps://in.xfel | ttps://in.xfel | ttps://in.xfel |
| P | .eu/calibratio | .eu/calibratio | .eu/calibratio | .eu/calibratio |
| D | n/calibration_ | n/calibration_ | n/calibration_ | n/calibration_ |
| 0 | constant_versi | constant_versi | constant_versi | constant_versi |
| 1 | ons//87567>`__ | ons//87580>`__ | ons//87574>`__ | ons//92102>`__ |
+---+----------------+----------------+----------------+----------------+
| A | `2021-08-05    | `2021-08-05    | `2021-08-05    | `2021-08-09    |
| G | 16:24 <h       | 16:24 <h       | 16:24 <h       | 14:45 <h       |
| I | ttps://in.xfel | ttps://in.xfel | ttps://in.xfel | ttps://in.xfel |
| P | .eu/calibratio | .eu/calibratio | .eu/calibratio | .eu/calibratio |
| D | n/calibration_ | n/calibration_ | n/calibration_ | n/calibration_ |
| 0 | constant_versi | constant_versi | constant_versi | constant_versi |
| 2 | ons//87563>`__ | ons//87579>`__ | ons//87571>`__ | ons//92042>`__ |
+---+----------------+----------------+----------------+----------------+

      It is not displayed properly anyway in the notebook itself. So it is still WIP.

      Edited by Karim Ahmed
    • Philipp Schmidt
      Philipp Schmidt @schmidtp ·
      Owner

      Oh! Then that will be connected to the weird typesetting. A single character by line is not the same in LaTeX.

    • Karim Ahmed
      Karim Ahmed @ahmedk ·
      Author Owner

      Yes, but this is not the main issue for this MR. this is just what this draft MR state is at the moment after trying to go from pipe to grid. Which didn't really solve the issue.

    • Karim Ahmed
      Karim Ahmed @ahmedk ·
      Author Owner

      I added the current RST format for the Markdown table in the description

    • Thomas Kluyver
      Thomas Kluyver @kluyvert ·
      Owner

      Do we have any tables with links in that work already, from any of our code in notebooks?

    • Karim Ahmed
      Karim Ahmed @ahmedk ·
      Author Owner

      No

    • Karim Ahmed
      Karim Ahmed @ahmedk ·
      Author Owner 
      +---------+--------------+--------------+--------------+--------------+
| Modules | BadPi        | Offset10Hz   | Rela         | Bad          |
|         | xelsDark10Hz |              | tiveGain10Hz | PixelsFF10Hz |
+=========+==============+==============+==============+==============+
| JNGFR03 | `2021-09-19  | `2021-09-19  | `2021-04-14  | `2021-04-14  |
|         | 13:27 <      | 13:27 <      | 20:05        | 20:05        |
|         | https://in.x | https://in.x | <https://in. | <https://in. |
|         | fel.eu/calib | fel.eu/calib | xfel.eu/cali | xfel.eu/cali |
|         | ration/calib | ration/calib | bration/cali | bration/cali |
|         | ration_const | ration_const | bration_cons | bration_cons |
|         | ant_versions | ant_versions | tant_version | tant_version |
|         | //102593>`__ | //102591>`__ | s//72819>`__ | s//72820>`__ |
+---------+--------------+--------------+--------------+--------------+

      So I know the problem, but not the solution.

      In this example, RelativeGain10Hz and BadPixelsFF10Hz will be displayed fine.

      if the table looks like this

      +---------+--------------+--------------+--------------+--------------+
| Modules | BadPi        | Offset10Hz   | Rela         | Bad          |
|         | xelsDark10Hz |              | tiveGain10Hz | PixelsFF10Hz |
+=========+==============+==============+==============+==============+
| JNGFR03 | `2021-09-19  | `2021-09-19  | `2021-04-14  | `2021-04-14  |
|         | 13:27        | 13:27        | 20:05        | 20:05        |
|         | <https://in.x| <https://in.x| <https://in. | <https://in. |
|         | fel.eu/calib | fel.eu/calib | xfel.eu/cali | xfel.eu/cali |
|         | ration/calib | ration/calib | bration/cali | bration/cali |
|         | ration_const | ration_const | bration_cons | bration_cons |
|         | ant_versions | ant_versions | tant_version | tant_version |
|         | //102593>`__ | //102591>`__ | s//72819>`__ | s//72820>`__ |
+---------+--------------+--------------+--------------+--------------+

      Then all the tables will look fine. The difference is the the link is enclosed with the < > on the same line row.

      Edited by Karim Ahmed
    • Karim Ahmed
      Karim Ahmed @ahmedk ·
      Author Owner

      • What I was looking at is if I can increase the width of the table somehow when it is converted to RST

      • or have the table be converted to RST and be shown properly in the notebook with a different representation.

      But so far I haven't reached proper solutions.

    • Please register or sign in to reply
  • Karim Ahmed changed the description

    changed the description

  • Karim Ahmed added 1 commit

    added 1 commit

    • a695da34 - latex table instead of markdown

    Compare with previous version

  • Karim Ahmed
    Karim Ahmed @ahmedk started a thread on an old version of the diff
notebooks/Jungfrau/Jungfrau_Gain_Correct_and_Verify_NBC.ipynb
223 223 " constant_names += [\"BadPixelsFF10Hz\", \"RelativeGain10Hz\"]\n",
224 224 " jf_metadata = jf_cal.metadata(calibrations=constant_names) \n",
225 225 " # Display retrieved calibration constants timestamps\n",
226 " jf_cal.display_markdown_retrieved_constants(metadata=jf_metadata)\n",
226 " jf_cal.display_latex_retrieved_constants(metadata=jf_metadata)\n",
  • Karim Ahmed
    Karim Ahmed @ahmedk ·
    Author Owner

    This function is another alternative that shows a proper table in the pdf like expected, however, the Latex table in the notebook will look like this:

    Screenshot_from_2024-07-12_12-51-23

    I have tested with html as well but it doesnt get rendered into the pdf

    Edited by Karim Ahmed
  • Please register or sign in to reply
  • Karim Ahmed added 1 commit

    added 1 commit

    Compare with previous version

  • Karim Ahmed added 1 commit

    added 1 commit

    • bce3f16b - feat: switch between latex and markdown based on environment variable NB_CONVERT_RUNNING

    Compare with previous version

  • Karim Ahmed marked this merge request as ready

    marked this merge request as ready

  • Karim Ahmed added 29 commits

    added 29 commits

    • bce3f16b...ffd7d60a - 25 commits from branch master
    • dbb13c9f - fix: CCV markdown tablefmt from pipe to grid for a better rst conversion and pdf display
    • 630ecf15 - latex table instead of markdown
    • 0235d15d - clean up the latex function
    • 2fd52349 - feat: switch between latex and markdown based on environment variable NB_CONVERT_RUNNING

    Compare with previous version

    Toggle commit list
  • Karim Ahmed marked the checklist item Jungfrau as completed

    marked the checklist item Jungfrau as completed

  • Karim Ahmed changed the description

    changed the description

  • Karim Ahmed added 1 commit

    added 1 commit

    • 238b0303 - fix: update the rest of correction notebook withe new function

    Compare with previous version

  • Karim Ahmed changed the description

    changed the description

  • Karim Ahmed marked the checklist item AGIPD as completed

    marked the checklist item AGIPD as completed

  • Karim Ahmed marked the checklist item pnCCD as completed

    marked the checklist item pnCCD as completed

  • Karim Ahmed marked the checklist item epix100 as completed

    marked the checklist item epix100 as completed

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
    • Please register or sign in to reply

    Legal Notice | Contacts