Pre and post bug bash fixes#66
Conversation
paleolimbot
left a comment
paleolimbot
left a comment
There was a problem hiding this comment.
Thank you! A few optional comments for now or later!
paleolimbot
left a comment
paleolimbot
left a comment
There was a problem hiding this comment.
Look great! In CI I see:
WARNING - A reference to 'quickstart-python.md' is included in the 'nav' configuration, which is not found in the documentation files.
Should it be included or removed?
Thanks for catching that. I ran |
as I was being too eager and was going to cause conflicts with somebody else's PR, such as apache#66
paleolimbot
left a comment
paleolimbot
left a comment
There was a problem hiding this comment.
Nice!
Just a few technical questions on my end. I'd like to move towards rendering the notebook content automatically (i.e., jupyter nbconvert --to markdown --execute path/to/content.ipynb), so some of the comments are just making sure that when we do that we'll get your updates!
jesspav
left a comment
jesspav
left a comment
There was a problem hiding this comment.
These changes look great!
I addd one question in line.
|
@kadolor CI failed. Please see the error message |
Co-authored-by: Dewey Dunnington <dewey@dunnington.ca>
|
@kadolor still failed |
| sd = sedona.db.connect() | ||
|
|
||
| df = sd.read_parquet( | ||
| 's3://wherobots-benchmark-prod/SpatialBench_sf=1_format=parquet/' |
There was a problem hiding this comment.
I guess this line can be removed
paleolimbot
left a comment
paleolimbot
left a comment
There was a problem hiding this comment.
Thanks!
I have a few comments on repo-level items that need to be solved before this merges...I'd like to get this merged (even if not perfect) and iterate with smaller changes.
I will set up the .ipynb to .md rendering script in a follow-up PR...as long as you are happy with the content here and are confident that the markdown files reflect the latest notebooks, I'm happy too!
|
@paleolimbot I think this PR is ready to be merged. |
paleolimbot
left a comment
paleolimbot
left a comment
There was a problem hiding this comment.
Thank you!
This does need a CI run (perhaps @jiayuasu can start one) but the content looks great to me!
5 participants
No description provided.