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

New option --use-source-timestamp #790

Merged
merged 3 commits into from May 29, 2021
Merged

New option --use-source-timestamp #790

merged 3 commits into from May 29, 2021

Conversation

mwouts
Copy link
Owner

@mwouts mwouts commented May 20, 2021

Fixes #784

@mwouts
Copy link
Owner Author

mwouts commented May 20, 2021

Hi @DancingQuanta, @Midnighter, would you like to give a try at this PR that introduces a new --use-source-timestamp argument? Does it improve your experience of using jupytext together with make?

NB: to install the version locally, use

pip install git+https://github.com/mwouts/jupytext.git@use_source_timestamp

See https://jupytext.readthedocs.io/en/latest/developing.html for more details. Thanks!

@codecov
Copy link

codecov bot commented May 20, 2021

Codecov Report

Merging #790 (2b2abc7) into master (1d63611) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #790      +/-   ##
==========================================
+ Coverage   99.11%   99.13%   +0.01%     
==========================================
  Files         106      106              
  Lines       10100    10138      +38     
==========================================
+ Hits        10011    10050      +39     
+ Misses         89       88       -1     
Impacted Files Coverage Δ
jupytext/cli.py 96.17% <100.00%> (+0.08%) ⬆️
tests/test_cli.py 100.00% <100.00%> (ø)
jupytext/paired_paths.py 98.49% <0.00%> (+0.75%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d63611...2b2abc7. Read the comment docs.

@Midnighter
Copy link

Midnighter commented May 23, 2021

Thank you for the quick response @mwouts.

I've tried this branch and the --use-source-timestamp works as advertised for me.

I also tried it without that flag, jupytext --to ipynb notebook.py, with the py:percent and ipynb files not being in sync and it doesn't update the timestamp of the .py file any longer while generating the .ipynb with the current time. This is what I needed for my workflow. Thanks a lot again.

Adjust 'formats', if required, before writing the notebook
@mwouts
Copy link
Owner Author

mwouts commented May 29, 2021

Thank you @Midnighter for your feedback! Glad it works as expected 😄 I'll fix (I mean... ignore) the sub-micro second timestamp issue on mac os (https://github.com/mwouts/jupytext/pull/790/checks?check_run_id=2702174937) and merge this

dest_timestamp = test_ipynb.stat().mtime
>       assert src_timestamp == dest_timestamp
E       assert 1622323297.7878923 == 1622323297.787892

@mwouts mwouts merged commit 112ed80 into master May 29, 2021
@mwouts mwouts deleted the use_source_timestamp branch May 29, 2021 21:58
@@ -1335,7 +1335,8 @@ def test_use_source_timestamp(tmpdir, cwd_tmpdir, python_notebook, capsys, forma
assert "Updating the timestamp" not in capture.out

dest_timestamp = test_ipynb.stat().mtime
assert src_timestamp == dest_timestamp
# on Mac OS the dest_timestamp is truncated at the microsecond (#790)
assert src_timestamp - 1e-6 <= dest_timestamp <= src_timestamp

Choose a reason for hiding this comment

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

Since you use pytest, you could use pytest.approx here.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Oh that's right... I like very much pytest.approx indeed! But here maybe the micro-second truncation is a bit more specific than a numerical uncertainty? Anyway thanks for commenting!

Choose a reason for hiding this comment

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

Fair point!

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.

update timestamp of destination files not source files
2 participants