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

Fix local docs table gen #1593

Closed
wants to merge 3 commits into from

Conversation

ThomasHepworth
Copy link
Contributor

Type of PR

  • BUG
  • FEAT
  • MAINT
  • DOC

Is your Pull Request linked to an existing Issue or Pull Request?

Tags onto #1590.

Give a brief description for the solution you have provided

Fixes an issue (at least for me) in which the generated docs tables (splink_datasets, comparison_library, etc.) weren't being generated locally.

This would then cause the if statement if [[ ! -f "docs/includes/generated_files/comparison_level_library_dialect_table.md" ]] to continuously trigger if you were to run the docs builder repeatedly (which frustratingly installs all Splink dependencies through poetry).

The issue is with the way in which filepaths and imports are working in the table generation scripts. The fix below simply takes advantage of the poetry creation bash script. With the changes below, Splink is now built and then installed using poetry, rather than trying to use and resolve all of the splink dev dependencies before creating the docs.

This should be quicker, install less fluff and most importantly, fix the main issue causing the tables not to be created by installing Splink directly.

Local build looks like so:
Screenshot 2023-09-11 at 12 45 27

@github-actions
Copy link
Contributor

Test: test_2_rounds_1k_duckdb

Percentage change: -12.2%

date time stats_mean stats_min commit_info_branch commit_info_id machine_info_cpu_brand_raw machine_info_cpu_hz_actual_friendly commit_hash
849 2022-07-12 18:40:05 1.89098 1.87463 splink3 c334bb9 Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz 2.7934 GHz c334bb9
2034 2023-09-11 11:47:48 1.68472 1.64652 (detached head) 0d56a9f Intel(R) Xeon(R) Platinum 8171M CPU @ 2.60GHz 2.0952 GHz 0d56a9f

Test: test_2_rounds_1k_sqlite

Percentage change: -12.4%

date time stats_mean stats_min commit_info_branch commit_info_id machine_info_cpu_brand_raw machine_info_cpu_hz_actual_friendly commit_hash
851 2022-07-12 18:40:05 4.32179 4.25898 splink3 c334bb9 Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz 2.7934 GHz c334bb9
2036 2023-09-11 11:47:48 3.79053 3.72968 (detached head) 0d56a9f Intel(R) Xeon(R) Platinum 8171M CPU @ 2.60GHz 2.0952 GHz 0d56a9f

Click here for vega lite time series charts

@ThomasHepworth ThomasHepworth changed the title Fix custom docs tables local gen Fix local docs tables gen Sep 11, 2023
@ThomasHepworth ThomasHepworth changed the title Fix local docs tables gen Fix local docs table gen Sep 11, 2023
@ThomasHepworth
Copy link
Contributor Author

Just to confirm, I'm still getting👇 when sourcing the make docs bash scripts which installs our dependencies using poetry. This causes every subsequent call of this script to run poetry install for me.

The change in this PR eliminates a lot of the initial bloat (by only install the core dependencies) and for whatever reason, also causing the python scripts to trigger and create the files I need.

For reference, I am using python 3.11, but this has been a long standing issue for me.

Screenshot 2023-09-25 at 11 04 37

@RobinL RobinL deleted the fix_custom_docs_tables_local_gen branch August 12, 2024 10:06
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.

2 participants