Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Move packages/python/plotly to top level #5002

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

Merged
emilykl merged 10 commits into master from simplify-dir-structure
Feb 4, 2025
Merged

Conversation

@emilykl
Copy link
Contributor

@emilykl emilykl commented Jan 30, 2025
edited
Loading

  • Moves the contents of packages/python/plotly to the top level of the repository
  • Updates paths across the repository to correspond to the new location

The full diff is pretty unwieldy, Github does not perform well with large diffs.

Link to the diff which excludes the commit where files were moved (more manageable): https://github.com/plotly/plotly.py/pull/5002/files/32adcc3de1cf3f6d15b35a4ed85e981bc9a8b24b..54d076b7c734d99e8efe9a96491cbe6b09adbc3c

@emilykl emilykl force-pushed the simplify-dir-structure branch from a2c3bf5 to f642b58 Compare January 31, 2025 23:17
@emilykl emilykl changed the title (削除) move packages/python/plotly to top level (削除ここまで) (追記) Move packages/python/plotly to top level (追記ここまで) Feb 1, 2025
@gvwilson gvwilson added P1 needed for current cycle infrastructure build process etc. labels Feb 3, 2025
@emilykl emilykl marked this pull request as ready for review February 3, 2025 21:33
Copy link
Collaborator

So sad that github diff doesn't work with this PR 😭 I'll just leave comments on what I find here because I basically can't leave comments in the diff.

  • README.md could include a link to the plotly-geo repo since that has its own section there. Just in case people want to make issues or even just to track down where it is.
  • Looks like the .gitignore needs to be updated - specifically:
packages/javascript/jupyterlab-plotly/lib/
plotly/jupyterlab_plotly/labextension/
plotly/jupyterlab_plotly/nbextension/index.js*
  • I feel like we should remove the whole section on tox in the CONTRIBUTING.md but that's for a different PR

Overall, I think this is ready to merge even if there will be some finishing touches afterward.

emilykl reacted with thumbs up emoji

Copy link
Contributor

gvwilson commented Feb 4, 2025

@emilykl would you like to push the big green button or shall I?

Copy link
Contributor Author

emilykl commented Feb 4, 2025

@gvwilson I'll make the changes Martha suggested and then merge!

Copy link
Member

The make file for the API reference docs needs an update for the new structure:
https://github.com/plotly/plotly.py/blob/simplify-dir-structure/doc/apidoc/Makefile
It still references packages/python...

emilykl reacted with eyes emoji

Copy link
Contributor Author

emilykl commented Feb 4, 2025

Thanks @LiamConnors , good catch.

I've updated the paths -- how would you recommend I verify that the Makefile is correct now?

Copy link
Member

Thanks @LiamConnors , good catch.

I've updated the paths -- how would you recommend I verify that the Makefile is correct now?

If you install the requirements.txt in doc and then go to doc/apidoc, running make html should generate the output.
I've tested it with these changes and the API docs build correctly.

emilykl reacted with hooray emoji

@emilykl emilykl merged commit 5813a8a into master Feb 4, 2025
5 checks passed
@emilykl emilykl deleted the simplify-dir-structure branch February 4, 2025 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@marthacryan marthacryan marthacryan approved these changes

@LiamConnors LiamConnors LiamConnors approved these changes

@alexcjohnson alexcjohnson Awaiting requested review from alexcjohnson

+1 more reviewer

@gvwilson gvwilson gvwilson approved these changes

Reviewers whose approvals may not affect merge requirements

Labels

infrastructure build process etc. P1 needed for current cycle

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /