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

Upload a figure comparison web page from the figure tests #2660

Merged
merged 1 commit into from Jul 4, 2018

Conversation

Projects
None yet
4 participants
@dstansby
Copy link
Contributor

commented Jun 15, 2018

This is a proof of concept for uploading a html page that shows the images generated by tests, and the images generated by the current master branch side by side.

The page could look nicer, but I'm going to leave that to someone else who has more time/knowledge of html/css.

See https://309-104337970-gh.circle-artifacts.com/0/root/project/figure_test_images/fig_comparison.html for the generated page for this PR.

@pep8speaks

This comment has been minimized.

Copy link

commented Jun 15, 2018

Hello @dstansby! Thanks for updating the PR.

Line 74:101: E501 line too long (101 > 100 characters)

Comment last updated on July 03, 2018 at 16:58 Hours UTC
@sunpy-bot

This comment has been minimized.

Copy link

commented Jun 15, 2018

Thanks for the pull request @dstansby! Everything looks great!

@nabobalis nabobalis added this to the 1.0 milestone Jun 15, 2018

if len(new_hash_library) > 0:
# Write the new hash library in JSON
figure_base_dir = os.path.abspath(config.getoption("--figure_dir"))
hashfile = os.path.join(figure_base_dir, HASH_LIBRARY_NAME)
with open(hashfile, 'w') as outfile:
json.dump(new_hash_library, outfile, sort_keys=True, indent=4, separators=(',', ': '))

if ('CI' in os.environ) and bool(os.environ['CI']):

This comment has been minimized.

Copy link
@Cadair

Cadair Jun 15, 2018

Member

to be honest I would just do this everytime, would be useful locally as well.



@pytest.mark.remote_data
def download_baseline_images(figure_base_dir, baseline_fig_dir):

This comment has been minimized.

Copy link
@Cadair

Cadair Jun 15, 2018

Member

do we actually need to download these? can we not just link to the ones hosted on github?

This comment has been minimized.

Copy link
@dstansby

dstansby Jun 15, 2018

Author Contributor

🤦‍♂️ oh yeah that's how the internet works

@dstansby dstansby force-pushed the dstansby:circle-fig-artefacts branch 3 times, most recently from 2cda0f9 to 9b482b3 Jun 15, 2018

if len(new_hash_library) > 0:
# Write the new hash library in JSON
figure_base_dir = os.path.abspath(config.getoption("--figure_dir"))
hashfile = os.path.join(figure_base_dir, HASH_LIBRARY_NAME)
with open(hashfile, 'w') as outfile:
json.dump(new_hash_library, outfile, sort_keys=True, indent=4, separators=(',', ': '))

generate_figure_webpage()

This comment has been minimized.

Copy link
@nabobalis

nabobalis Jun 15, 2018

Contributor

Do we only want to generate the webpage if there is a difference?

This comment has been minimized.

Copy link
@dstansby

dstansby Jun 15, 2018

Author Contributor

Don't see why not, it's a small overhead and just generates a small .html file.

@dstansby
Copy link
Contributor Author

left a comment

This still needs

  • Update to some of the code to use more pathlib
html_file = pathlib.Path(figure_base_dir) / 'fig_comparison.html'
with open(str(html_file), 'w') as f:
f.write(html_intro)
for fname in os.listdir(figure_base_dir):

This comment has been minimized.

Copy link
@dstansby

dstansby Jun 16, 2018

Author Contributor

This and lines below can be made much better using pathlib.

@dstansby dstansby force-pushed the dstansby:circle-fig-artefacts branch from 9b482b3 to 03f1024 Jul 3, 2018

@Cadair

Cadair approved these changes Jul 3, 2018

Copy link
Member

left a comment

I mean pathlib is just amazeballs isn't it.

@Cadair

This comment has been minimized.

Copy link
Member

commented Jul 3, 2018

I should make sunpybot report this as a commit status if the build fails.

@dstansby

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2018

woah it all passed!

@Cadair

This comment has been minimized.

Copy link
Member

commented Jul 3, 2018

oh noes, codecov came for you...

@Cadair Cadair added the [Review] label Jul 3, 2018

@dstansby

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2018

Lets try again now I've delted my personal sunpy codecov repository.

Proff of concept circlci figure upload
Fix indentation

Generate html page on circle

~

Try marking remote data

Try an online flag

strrr

Moar str

Duh internet

Remove test

<3 pathlib

Add changelog

Fix plot saving

@dstansby dstansby force-pushed the dstansby:circle-fig-artefacts branch from c71eb6c to 3c0b67d Jul 3, 2018

@dstansby

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2018

Problem was I had my own circleci job, and circleci is clever and doesn't run an organisation job when there's an identical user job. Hopefully should work now.

@dstansby

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2018

I think the codecov changes are un-related a due to the travis build?

@nabobalis

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2018

@Cadair Cadair merged commit e728542 into sunpy:master Jul 4, 2018

7 of 9 checks passed

codecov/patch 40% of diff hit (target 84.2%)
Details
codecov/project 83.96% (-0.24%) compared to 79bd97b
Details
Giles Click details to preview the documentation build
Details
ci/circleci: egg-info-36 Your tests passed on CircleCI!
Details
ci/circleci: figure-tests-36 Your tests passed on CircleCI!
Details
ci/circleci: html-docs Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
sunpy-bot All checks passed

@dstansby dstansby deleted the dstansby:circle-fig-artefacts branch Jul 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.