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

remove custom log message editing (closes #1545) #1550

Merged
merged 2 commits into from
Apr 5, 2024

Conversation

petrelharp
Copy link
Contributor

I looked at this a fair bit, and tried to figure out why the LogRecord object that previously had non-empty args now has empty logs, and also how to have a unit test for this, but didn't manage to on both counts.

On the other hand, I don't actually think this code was actually doing anything:

$ python3 -m stdpopsim -e slim --slim-scaling-factor 10 HomSap -c chr22 --left 10000000 --right 10010000 -o foo1.ts -d Africa_1T12 AFR:100
Simulation information:
    Engine: slim (4.1)
    Model id: Africa_1T12
    Model description: African population
    Population: number_samples (sampling_time_generations):
        AFR: 100 (0)
Contig Description:
    Contig origin: chr22:10000000-10010000
    Contig length: 10000.0
    Contig ploidy: 2
    Mean recombination rate: 2.1057233894035443e-08
    Mean mutation rate: 2.36e-08
    Genetic map: None

WARNING: /home/peter/projects/popsim-consortium/stdpopsim/stdpopsim/slim_engine.py:1602: SLiMScalingFactorWarning: You're using a scaling factor (10.0). This should give similar results for many situations, but is not equivalent, especially in the presence of selection. When using rescaling, you should be careful---do checks and compare results across different values of the scaling factor.
  warnings.warn(

WARNING: /home/peter/projects/tskit-dev/treestuff_venv/lib/python3.11/site-packages/msprime/ancestry.py:831: TimeUnitsMismatchWarning: The initial_state has time_units=ticks but time is measured in generations in msprime. This may lead to significant discrepancies between the timescales. If you wish to suppress this warning, you can use, e.g., warnings.simplefilter('ignore', msprime.TimeUnitsMismatchWarning)
  warnings.warn(message, TimeUnitsMismatchWarning)

The odd "warnings.warn(" lines are merely printing the line in the source code where the warning is raised, but the code was not removing that entirely.

@petrelharp
Copy link
Contributor Author

Arrrgh.

@petrelharp
Copy link
Contributor Author

This should work after #1553 goes in.

Copy link

codecov bot commented Apr 5, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 99.83%. Comparing base (6926072) to head (2ad99df).

Files Patch % Lines
stdpopsim/cli.py 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1550   +/-   ##
=======================================
  Coverage   99.83%   99.83%           
=======================================
  Files         131      131           
  Lines        4333     4334    +1     
  Branches      594      595    +1     
=======================================
+ Hits         4326     4327    +1     
  Misses          3        3           
  Partials        4        4           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@petrelharp petrelharp merged commit 5f56d36 into popsim-consortium:main Apr 5, 2024
10 of 11 checks 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.

1 participant