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

Geoshape docs revamp 5.0 #2699

Merged
merged 27 commits into from
Nov 5, 2022
Merged

Geoshape docs revamp 5.0 #2699

merged 27 commits into from
Nov 5, 2022

Conversation

mattijn
Copy link
Contributor

@mattijn mattijn commented Oct 19, 2022

Same as #2688, but from direct fork from altair repo. Like this it should be easier to stay in-sync.

When I try to build the docs locally I get the following error:

-> saving /Users/mattijnvanhoek/mattijn/altair/doc/_images/scatter_linked_table.png
/Users/mattijnvanhoek/miniconda3/share/vega-lite-cli/node_modules/vega-lite/build/vega-lite.js:40
          n = p.length;
                ^

TypeError: Cannot read properties of undefined (reading 'length')

I don't think it is related to this PR, but I like to test locally. @joelostblom, did you face this issue before?

@joelostblom
Copy link
Contributor

joelostblom commented Oct 20, 2022

I just tried building the docs in a fresh env and new repo clone on the master branch and I run into the same error. I tried to troubleshoot a bit but I can't figure out what is wrong. I vaguely remember encountering that or something similar before, but I believe that the fix then was to install altair_saver via pip and then do the npm install... step. When I try that now I get an error from npm that I don't have time to dive into unfortunately.

It's especially odd as I am on an Ubuntu box and follow the same installation steps as in our CI workflow which is also ubuntu-based... https://github.com/altair-viz/altair/blob/master/.github/workflows/docbuild.yml

@joelostblom
Copy link
Contributor

Ok so it worked for me to rely on selenium instead of the vega packages via npm. I had to run the following to install chrome driver, I guess the GitHub CI Ubuntu image comes with this package pre-installed.

apt-get install chromium-chromedriver

@mattijn
Copy link
Contributor Author

mattijn commented Oct 20, 2022

Ah, yeah. Thanks🙏!

@mattijn
Copy link
Contributor Author

mattijn commented Nov 2, 2022

Building docs locally actually works with vl-convert-python🎉


Few debug messages during doc building that require some attention

(base) mattijnvanhoek@Mattijns-MacBook-Pro doc % make html
WARNING: [autosummary] failed to import altair.TopLevelParameter.
Possible hints:
* AttributeError: module 'altair' has no attribute 'TopLevelParameter'
* ImportError: 
* ModuleNotFoundError: No module named 'altair.TopLevelParameter'

/Users/mattijnvanhoek/miniconda3/lib/python3.9/site-packages/recommonmark/parser.py:75: UserWarning: Container node skipped: type=document
  warn("Container node skipped: type={0}".format(mdnode.t))

/Users/mattijnvanhoek/mattijn/altair/altair/vegalite/v5/api.py:331: AltairDeprecationWarning: Use 'value' instead of 'init'.
  warnings.warn(
<string>:3: UserWarning: Geometry is in a geographic CRS. Results from 'centroid' are likely incorrect. Use 'GeoSeries.to_crs()' to re-project geometries to a projected CRS before this operation.

The HTML pages are in _build/html.

@joelostblom
Copy link
Contributor

That's great! I'm looking forward to this being the new default in Altair. I will try to look into this (and the other) PR at the end of next week, sorry for the long review time, just being swamped with things at work at the moment.

Copy link
Contributor

@joelostblom joelostblom left a comment

Choose a reason for hiding this comment

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

I didn't want to hold you up, so I found some time to work on this. As I said before in the other repo: I love these docs! It's exciting to see how much easier it is to work with Altair using geopandas like this, and it solves many of the issues I have had in the past. I like the advanced sections you have too, they are great for people who wants to go further.

doc/user_guide/marks/index.rst Outdated Show resolved Hide resolved
doc/user_guide/marks/geoshape.rst Outdated Show resolved Hide resolved
doc/user_guide/marks/geoshape.rst Outdated Show resolved Hide resolved
doc/user_guide/marks/geoshape.rst Outdated Show resolved Hide resolved
doc/user_guide/marks/geoshape.rst Outdated Show resolved Hide resolved
doc/user_guide/marks/geoshape.rst Outdated Show resolved Hide resolved
doc/user_guide/marks/geoshape.rst Outdated Show resolved Hide resolved
doc/user_guide/marks/geoshape.rst Outdated Show resolved Hide resolved
doc/user_guide/marks/geoshape.rst Outdated Show resolved Hide resolved
doc/user_guide/marks/geoshape.rst Show resolved Hide resolved
@joelostblom
Copy link
Contributor

Also a headsup that I see a lot of this warning when buildingthe worldmap example in the docs:

-> saving /home/joel/Downloads/altair/doc/_images/world_projections.png
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 45 source files that are out of date
updating environment: [new config] 735 added, 0 changed, 0 removed
/home/joel/miniconda3/envs/geoalt/lib/python3.10/site-packages/recommonmark/parser.py:75: UserWarning: Container node skipped: type=document
  warn("Container node skipped: type={0}".format(mdnode.t))
/home/joel/miniconda3/envs/geoalt/lib/python3.10/site-packages/recommonmark/parser.py:75: UserWarning: Container node skipped: type=document
  warn("Container node skipped: type={0}".format(mdnode.t))
/home/joel/miniconda3/envs/geoalt/lib/python3.10/site-packages/recommonmark/parser.py:75: UserWarning: Container node skipped: type=document
  warn("Container node skipped: type={0}".format(mdnode.t))

mattijn and others added 12 commits November 5, 2022 15:44
Co-authored-by: Joel Ostblom <joelostblom@users.noreply.github.com>
Co-authored-by: Joel Ostblom <joelostblom@users.noreply.github.com>
Co-authored-by: Joel Ostblom <joelostblom@users.noreply.github.com>
Co-authored-by: Joel Ostblom <joelostblom@users.noreply.github.com>
Co-authored-by: Joel Ostblom <joelostblom@users.noreply.github.com>
Co-authored-by: Joel Ostblom <joelostblom@users.noreply.github.com>
Co-authored-by: Joel Ostblom <joelostblom@users.noreply.github.com>
Co-authored-by: Joel Ostblom <joelostblom@users.noreply.github.com>
Co-authored-by: Joel Ostblom <joelostblom@users.noreply.github.com>
Co-authored-by: Joel Ostblom <joelostblom@users.noreply.github.com>
Co-authored-by: Joel Ostblom <joelostblom@users.noreply.github.com>
Co-authored-by: Joel Ostblom <joelostblom@users.noreply.github.com>
mattijn and others added 2 commits November 5, 2022 16:12
Co-authored-by: Joel Ostblom <joelostblom@users.noreply.github.com>
Co-authored-by: Joel Ostblom <joelostblom@users.noreply.github.com>
@ChristopherDavisUCI
Copy link
Contributor

I know this is a dumb question, but is there a place for me to see these compiled docs?! I've never tried using any of this "geography" functionality before.

@mattijn mattijn merged commit 9e44cee into vega:master Nov 5, 2022
@mattijn
Copy link
Contributor Author

mattijn commented Nov 6, 2022

I know this is a dumb question, but is there a place for me to see these compiled docs?! I've never tried using any of this "geography" functionality before.

I use the method as is documented here https://github.com/altair-viz/altair/blob/master/RELEASING.md:

  1. Make certain your branch is in sync with head

    $ git pull upstream master
    
  2. Do a clean doc build:

    $ cd doc
    $ make clean-all
    $ make html
    $ cd _build/html; python -m http.server
    

    Navigate to http://localhost:8000

Make sure you have vl-convert-python installed, that will make it more likely that you successfully build the docs locally. Building docs takes time btw..

@binste
Copy link
Contributor

binste commented Dec 17, 2022

Also a headsup that I see a lot of this warning when buildingthe worldmap example in the docs:

-> saving /home/joel/Downloads/altair/doc/_images/world_projections.png
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 45 source files that are out of date
updating environment: [new config] 735 added, 0 changed, 0 removed
/home/joel/miniconda3/envs/geoalt/lib/python3.10/site-packages/recommonmark/parser.py:75: UserWarning: Container node skipped: type=document
  warn("Container node skipped: type={0}".format(mdnode.t))
/home/joel/miniconda3/envs/geoalt/lib/python3.10/site-packages/recommonmark/parser.py:75: UserWarning: Container node skipped: type=document
  warn("Container node skipped: type={0}".format(mdnode.t))
/home/joel/miniconda3/envs/geoalt/lib/python3.10/site-packages/recommonmark/parser.py:75: UserWarning: Container node skipped: type=document
  warn("Container node skipped: type={0}".format(mdnode.t))

I'll address this in #2758

@mattijn mattijn deleted the geoshape-docs branch January 4, 2023 10:41
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.

4 participants