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

587 remove frozen #166

Merged
merged 13 commits into from
Apr 4, 2024
Merged

587 remove frozen #166

merged 13 commits into from
Apr 4, 2024

Conversation

jwestw
Copy link
Contributor

@jwestw jwestw commented Dec 14, 2023

Pull Request submission

Removes the word frozen from the pipeline in both paths and in dictionary and Toml keys.

Closes or fixes

  • Detail the ticket(s) you are closing with this PR
    RDRP-587

Code

  • Code runs The code runs on my machine and/or CDSW
  • Conflicts resolved There are no conflicts (I have performed a rebase if necessary)
  • Requirements My/our code functions according to the requirements of the ticket
  • Dependencies I have updated the environment yaml so it includes any new libraries I have used
  • Configuration file updated any high level parameters that the user may interact with have been put into the config file (and imported to the script)
  • Clean Code
    • Code is as PEP 8 compliant as I can humanly make it
    • Code passess flake8 linting check
    • Code adheres to DRY
  • Type hints All new functions have type hints

Documentation

Any new code includes all the following forms of documentation:

  • Function Documentation Docstrings within the function(s')/methods have been created
    • Includes Args and returns for all major functions
    • The docstring details data types
  • Updated Documentation: User and/or developer working doc has been updated

Data

  • All data needed to run this script is available in Dev/Test
  • All data is excluded from this pull request
  • Secrets checker pre-commit passes

Testing

  • Unit tests Unit tests have been created and are passing or a new ticket to create tests has been created

Peer Review Section

  • All requirements install from (updated) environment.yaml
  • Documentation has been created and is clear - check the working document
  • Doctrings (Google format) have been created and accurately describe the function's functionality
  • Unit tests pass, or if not present a new ticket to create tests has been created
  • Code runs The code runs on reviewer's machine and/or CDSW

Final approval (post-review)

The author has responded to my review and made changes to my satisfaction.

  • I recommend merging this request.

Review comments

Insert detailed comments here!

These might include, but not exclusively:

  • bugs that need fixing (does it work as expected? and does it work with other code
    that it is likely to interact with?)
  • alternative methods (could it be written more efficiently or with more clarity?)
  • documentation improvements (does the documentation reflect how the code actually works?)
  • additional tests that should be implemented (do the tests effectively assure that it
    works correctly?)
  • code style improvements (could the code be written more clearly?)
  • Do the changes represent a change in functionality so the version number should increase? Start a discussion if so.
  • As a review you can generates the same outputs from running the code

Your suggestions should be tailored to the code that you are reviewing.
Be critical and clear, but not mean. Ask questions and set actions.

Copy link

github-actions bot commented Dec 14, 2023

Percentage Coverage for this PR

Detailed Coverage Report
FileStmtsMissCoverMissing
src
   __init__.py00100% 
src/aggregation
   __init__.py00100% 
src/construction
   __init__.py00100% 
   construction.py85850%2–4, 6–8, 10, 13, 40–43, 45–47, 50–52, 55–64, 67, 69, 72, 75, 78–79, 82–84, 87, 91–92, 97–98, 102–103, 105, 108, 110, 113, 116–117, 120, 128, 136–137, 139, 144, 146–147, 152, 155, 160–162, 165, 168, 170, 174, 176, 179, 184, 189, 196–198, 201–202, 206, 209–213, 216, 218, 220
   old_construction.py1131130%3–7, 10, 13, 41–45, 48–50, 53–55, 60, 63, 65, 68, 74–75, 88–89, 92, 101, 103–104, 107, 117, 120, 124, 132, 135–136, 138, 140–141, 144, 149, 156, 164, 167, 171–174, 176–177, 180, 183–187, 192, 199–202, 204, 207, 210–213, 216–217, 220–222, 224–229, 233–239, 244–245, 248–249, 252, 254–255, 257, 259, 263–264, 267, 270, 275, 278–280, 282, 286, 289, 292, 295–297, 301, 304, 306
src/estimation
   __init__.py00100% 
   apply_weights.py16160%2–5, 7, 10, 24–29, 32–34, 36
   calculate_weights.py37370%1–3, 6, 9, 23–25, 28, 30, 33, 64–66, 69, 72, 75–78, 80–81, 85, 96, 99, 102, 105, 108, 111, 114–115, 121–122, 124, 127, 139–140
   cellno_mapper.py70100% 
src/imputation
   MoR.py1121120%2–5, 7–9, 16–17, 20, 38, 41, 43–44, 46, 50, 52–53, 55, 58, 67–70, 73, 77–78, 80–82, 84, 86, 89, 91, 94, 119, 123, 126–129, 131, 134, 137, 142–144, 147, 150–151, 154–155, 159, 161, 164, 170–172, 175, 184, 188, 194, 201, 211–213, 217, 220, 229–230, 233, 241, 244, 246–248, 250, 254, 262, 264, 266, 268, 270–271, 277, 279–281, 283–284, 288–289, 294–295, 298, 309, 315, 318, 321, 323–324, 326, 329, 332–334, 336, 339, 341, 345, 348–349
   __init__.py00100% 
   apportionment.py361072%124, 126, 141, 143, 145, 157–159, 162, 164
   expansion_imputation.py393217%21, 25–26, 28–29, 32, 35, 38–40, 44, 47, 49, 52–53, 55, 58, 61, 81–83, 87, 90, 93, 96, 102, 107, 110, 112, 118, 121, 125
   imputation_helpers.py1014951%23–25, 27–28, 30, 32, 37, 50–53, 56–57, 59–60, 62–63, 65–66, 68–69, 71–72, 74–75, 77, 84–85, 87, 256, 258, 260–261, 263, 267, 273, 276, 280, 283–284, 286, 312, 315, 318, 327–328, 330, 332
   impute_civ_def.py944848%134–136, 166, 168–169, 172–173, 176–178, 183–184, 186, 189, 191–194, 196, 198, 203–204, 206, 209, 211–213, 216, 218–219, 221–222, 224–225, 238, 241–245, 247–248, 251–252, 254–255, 257
   manual_imputation.py19190%1–2, 4, 6, 9–10, 28–29, 33, 35, 37, 44, 59, 61–62, 64–65, 67–68
   pg_conversion.py36975%51, 54, 57, 121, 124, 127, 156, 159, 161
   sf_expansion.py72720%2–4, 6–7, 9, 11–12, 15, 24, 26, 29, 32–33, 36–37, 40, 43–44, 47, 49, 56–57, 61, 64, 69–70, 73, 75, 78–79, 81, 83, 86, 90, 92, 96, 104, 107, 111, 113–114, 116, 118, 121, 124, 133, 136, 139, 148, 151, 153, 159, 165, 168, 177, 180, 185–186, 188, 191–192, 196–197, 200, 204, 208, 211, 219, 221, 225, 227
   short_to_long.py21210%1, 3–4, 7, 20, 23, 25, 32–33, 35, 37–38, 40–41, 43–45, 47, 49, 53, 55
   tmi_imputation.py17814120%92, 94, 100–102, 105, 112, 115, 117, 122, 127, 132, 145, 148–149, 151, 154–155, 176–178, 180, 182, 185, 188–190, 192–193, 196, 199, 206–209, 211, 235, 238, 240, 243, 247, 251, 272–273, 276, 279–280, 283, 286–287, 289–290, 292, 294, 297, 299, 301, 304, 307–308, 310, 312–314, 316, 323, 325, 332, 334–335, 337–338, 341, 343, 345, 347, 350, 353–354, 357, 359, 361, 366, 372, 378, 384, 388, 407–408, 410, 412–413, 415–416, 418–419, 422, 424–425, 428, 431, 433–434, 450, 452, 455–457, 459, 461, 463–464, 467, 469–470, 473, 475, 477–478, 496, 500, 503–504, 507–508, 511, 514, 516, 519, 521, 526, 528, 535–536, 539, 542–543, 545, 548, 550–551
src/northern_ireland
   __init__.py00100% 
   ni_headcount_fte.py29290%2–4, 6, 8, 11, 25, 27, 29, 31, 33, 35, 37, 40, 55, 57–59, 61, 63–64, 66–67, 69, 72, 81, 83–84, 86
   ni_staging.py36360%3–6, 8, 10, 13, 21–22, 25, 27–29, 31, 34, 40, 44, 46, 49, 80–81, 84, 89, 93, 96, 99–107, 109, 111
src/outlier_detection
   __init__.py00100% 
   auto_outliers.py830100% 
   manual_outliers.py160100% 
src/outputs
   __init__.py00100% 
   export_files.py1011010%5–11, 13–14, 18, 24, 42, 44, 51, 56, 63, 68, 71, 98, 104, 107, 114, 116, 119, 124–125, 128–132, 135–136, 139, 151–153, 155, 158, 169, 171–172, 174, 177, 195, 198, 201–202, 209, 213–214, 217–219, 222, 224, 226–235, 239, 241–250, 255–256, 258, 261–263, 266, 269, 272, 286, 289–290, 298, 301, 303, 305, 315, 317–319, 328, 330, 333–334
   form_output_prep.py23230%1–4, 7, 33–34, 38–39, 41–42, 44, 46–51, 54, 59, 61, 65, 67
   frozen_group.py40400%3–4, 6–7, 9–11, 13, 16, 47–49, 51–52, 55, 62, 74, 82, 89, 92, 95–97, 100, 103–104, 111, 114, 117, 122–123, 126–127, 130–132, 135–137, 139
   gb_sas.py26260%2–5, 7–9, 11, 14, 34–36, 39, 44, 47, 50, 53, 56, 59, 64, 67–69, 72–74
   intram_by_civil_defence.py26260%2–6, 8, 11, 14, 34–36, 38–39, 42–44, 46, 49, 54, 57, 60–62, 65–67
   intram_by_itl1.py38380%2–5, 7–8, 10, 13, 36–38, 41, 44–47, 50–52, 55–56, 59–62, 65–66, 69, 72, 75, 78–82, 87–89
   intram_by_pg.py27270%2–5, 7, 10, 13, 31–33, 36–38, 40, 43–45, 48, 51, 54, 57–60, 65–67
   intram_by_sic.py36360%2–5, 7, 10, 13, 32–34, 36, 39–40, 43–45, 47, 50–52, 57, 73–77, 79, 82, 85, 88, 91, 94–95, 98–100
   intram_uk_itl_1_2.py33330%2–5, 7–8, 10, 13, 40–42, 45–48, 51, 54, 57, 62, 65, 71, 74, 77–78, 81, 84, 87–90, 99–101
   long_form.py21210%2–5, 7–9, 11, 14, 33–35, 38, 41, 44, 47–49, 51–53
   manifest_output.py78780%1–4, 8, 11–12, 15, 33, 48–51, 54–55, 59–60, 65–66, 68, 71–75, 78–84, 86, 104–105, 112, 114–115, 122, 125, 127, 129, 131, 135, 145, 150–151, 157, 160–161, 163–164, 172–175, 182, 189, 191, 196, 198–200, 202–203, 205–206, 208–211, 213, 216, 218, 224–225, 228–229
   map_output_cols.py746512%21–22, 24, 27–28, 30–31, 34, 52–53, 55, 58, 61, 64, 71–75, 78, 82, 84, 86–87, 104, 114, 117, 122, 125–126, 129, 131, 146–147, 149, 154–156, 160, 179–180, 183, 186, 189, 192, 194, 196, 198, 200–201, 220, 222, 224–225, 228–229, 232–233, 251–252, 254, 256, 259, 262, 264
   ni_sas.py23230%2–8, 10, 13, 35–37, 40, 42, 45, 48, 53, 56–58, 61–63
   outputs_helpers.py241058%46, 53–55, 79–81, 84, 87, 89
   short_form.py391853%78, 85, 87, 110–112, 115, 118, 121, 124, 127, 130, 133–135, 137–139
   tau.py26260%2–8, 10, 13, 33–35, 40, 43, 46, 49, 52, 55, 58, 63, 66–68, 71–73
   total_fte.py14140%2–5, 8, 11, 24–25, 27–28, 34, 39–41
src/site_apportionment
   __init__.py00100% 
   output_status_filtered.py211147%16–17, 36, 39, 41–43, 45–47, 49
   site_apportionment.py1194661%58, 98, 100–106, 216–217, 220, 224, 227, 234, 239, 247, 264, 267, 270–272, 276, 291–292, 295–296, 298–299, 402–403, 467, 471, 474, 477, 480, 483, 494, 497, 500, 503, 506, 509, 512, 515, 517
src/staging
   __init__.py00100% 
   history_loader.py32293%42, 54
   spp_parser.py140100% 
   spp_snapshot_processing.py340100% 
   staging_helpers.py14411023%50–52, 54–56, 58, 68, 71, 96–102, 119, 122, 131–133, 136, 139–142, 144, 146, 165–166, 168–169, 171, 210, 213, 216, 219, 222, 225–226, 229, 232, 234, 236, 239, 242, 257–259, 261–263, 265–267, 271–272, 276, 278, 281, 283, 305–306, 310, 315–317, 339, 342, 344, 347, 352–354, 357–358, 361, 363, 368, 374, 397–398, 401, 406–407, 410, 413, 417, 424, 455–458, 460–461, 464–465, 501, 504–507, 510, 513, 516–519, 522, 524
   validation.py2246670%17–18, 73, 207, 209, 309–310, 337, 340, 387–388, 399, 423, 437–438, 444, 448, 456–457, 463–464, 503–504, 511, 513, 516, 518, 539, 541–542, 545–546, 549–551, 553, 555, 558–559, 561, 563–564, 635, 637–638, 641–642, 645–646, 649, 652–653, 656, 658, 660–661, 674, 676–682, 685, 688
src/utils
   __init__.py00100% 
   helpers.py17570%14–15, 19–20, 22
   local_file_mods.py1054458%33–38, 83, 133–135, 183–184, 195–199, 210–211, 222, 233, 244–245, 247, 258–259, 270–271, 280, 288, 299, 301–302, 304–305, 309–311, 315, 329–330, 333–334, 336
TOTAL2489171830% 

Summary of tests

Tests Skipped Failures Errors Time
79 0 💤 0 ❌ 0 🔥 1.678s ⏱️

@jwestw jwestw marked this pull request as ready for review December 14, 2023 15:26
JenCheshire
JenCheshire previously approved these changes Dec 18, 2023
Copy link
Collaborator

@JenCheshire JenCheshire left a comment

Choose a reason for hiding this comment

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

Look good - just a couple of (very) minor suggestions

src/developer_config.yaml Outdated Show resolved Hide resolved
src/pipeline.py Outdated Show resolved Hide resolved
src/outputs/gb_sas.py Outdated Show resolved Hide resolved
@JenCheshire JenCheshire self-requested a review January 8, 2024 10:10
@AnneONS AnneONS dismissed JenCheshire’s stale review January 8, 2024 18:08

more changes required, removing approval

@jwestw
Copy link
Contributor Author

jwestw commented Jan 9, 2024

@AnneONS @JenCheshire @zorge69 I think everything is resolved now.

@jwestw jwestw requested a review from zorge69 January 9, 2024 16:34
Copy link
Collaborator

@JenCheshire JenCheshire left a comment

Choose a reason for hiding this comment

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

Looks good to me

@jwestw jwestw requested a review from JenCheshire January 16, 2024 13:17
Copy link
Collaborator

@JenCheshire JenCheshire left a comment

Choose a reason for hiding this comment

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

One question re: doubling up NI staging in pipeline.py (see below), and a change to the config - otherwise all looking good

src/pipeline.py Outdated Show resolved Hide resolved
src/developer_config.yaml Outdated Show resolved Hide resolved
@zorge69 zorge69 marked this pull request as draft February 15, 2024 10:11
@JenCheshire JenCheshire marked this pull request as ready for review March 18, 2024 14:47
@JenCheshire JenCheshire requested a review from AnneONS April 4, 2024 09:21
Copy link
Collaborator

@AnneONS AnneONS left a comment

Choose a reason for hiding this comment

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

This is now ready to merge. I've tested running it.

@AnneONS AnneONS merged commit 979d88d into develop Apr 4, 2024
1 check passed
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.

4 participants