Skip to content

Fix docs workflow failure due to Fiona package #2008

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.

Sign up for GitHub

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

Closed
Conengmo wants to merge 13 commits into from
Closed

Fix docs workflow failure due to Fiona package #2008

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c624941
import fiona
Conengmo Oct 17, 2024
26ed1d4
update heatmap test image
Conengmo Oct 17, 2024
6c37173
print canvas size
Conengmo Oct 18, 2024
408db0c
add fiona to main req
Conengmo Oct 18, 2024
17a4ea2
explicitely install fiona
Conengmo Oct 18, 2024
e630df5
use new chrome headless mode
Conengmo Oct 18, 2024
35c0320
try fiona as req-dev again
Conengmo Oct 18, 2024
2a9fc6e
try another window size
Conengmo Oct 18, 2024
97576af
restore window size, try another flag
Conengmo Oct 18, 2024
8f15043
install fiona explicitely
Conengmo Oct 18, 2024
9c10eb1
remove experimental selenium flags
Conengmo Oct 19, 2024
6343237
use a fixed figure size and crop the screenshot
Conengmo Oct 19, 2024
70a01c0
Merge remote-tracking branch 'upstream/main' into fix-test-issues
Conengmo Oct 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/deploy-docs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ jobs:
--file requirements.txt
--file requirements-dev.txt

# For some reason Fiona doesn't get installed from requirements-dev...
- name: Install fiona dependency
shell: bash -l {0}
run: python -m pip install fiona
Copy link
Member

@ocefpaf ocefpaf Oct 18, 2024

Choose a reason for hiding this comment

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

If you can wait I'll take a look at this next week. Something is not right here...

Copy link
Member Author

@Conengmo Conengmo Oct 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

something's weird right! I can definitely wait, very interesting to figure out what's going on.

Note that #2007 is waiting for the fix to get the docs going again. Waiting a week is not an issue, but if it's much longer then maybe a quick fix now and looking into it later is better. Definitely not meant to rush you! More of an expectation thing.

I did also manage to fix the Selenium heatmap test issue in this PR, but I'll spin that out in a separate one and merge that earlier.

Copy link
Collaborator

@hansthen hansthen Oct 19, 2024

Choose a reason for hiding this comment

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

I split the underlying issues for the Selenium heatmap and this change. Once we have both fixed shall we make a new release?

Copy link
Member Author

@Conengmo Conengmo Oct 19, 2024

Choose a reason for hiding this comment

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

sounds good!

Copy link
Member

@ocefpaf ocefpaf Oct 21, 2024

Choose a reason for hiding this comment

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

something's weird right! I can definitely wait, very interesting to figure out what's going on.

Found it. See #2012. Basically we are limiting to what is on main to build/test the docs.


- name: Install folium from source
shell: bash -l {0}
run: python -m pip install -e . --no-deps --force-reinstall
Expand Down
1 change: 1 addition & 0 deletions requirements-dev.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ altair>=5.0.0
cartopy
check-manifest
descartes
fiona
flake8
flake8-builtins
flake8-comprehensions
Expand Down