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

Fix docs workflow failure due to Fiona package #2008

Closed
wants to merge 13 commits into from

Conversation

Conengmo
Copy link
Member

@Conengmo Conengmo commented Oct 17, 2024

Closes #2002

Add fiona as an explicit dev dependency.

It is needed in this example in the docs https://python-visualization.github.io/folium/latest/user_guide/geojson/geopandas_and_geo_interface.html#Use-any-object-with-__geo_interface__

The other issue is with the heatmap verification. Seems like Chrome added some UI element to the headless browser that changed the window size. So I updated the verification image to correspond with the new size.

But now the size differ between my local computer and the remove env. I guess we should either rewrite this test to not be dependent on screen size or scrap it altogether.

@Conengmo
Copy link
Member Author

Conengmo commented Oct 17, 2024

I added fiona as a dependency but it's not being installed in Miniconda... weird. Is that a Github Actions caching issue? I don't understand.

@Conengmo
Copy link
Member Author

Can't get Fiona to install from the requirements. Instead I just install it with Pip explicitely. Not the nicest solution but it does work.

# 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

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

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

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

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

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.

@Conengmo Conengmo changed the title Fix test issues Fix docs workflow failure due to Fiona package Oct 19, 2024
@Conengmo
Copy link
Member Author

Close this one in favor of #2012

@Conengmo Conengmo closed this Oct 21, 2024
@Conengmo Conengmo deleted the fix-test-issues branch October 21, 2024 13:03
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.

Publish docs test fails due to missing fiona dependency
3 participants