Skip to content

Conversation

@stevenhua0320
Copy link
Contributor

@zmx27 ready for first round review
I need to do: check build doc after PR #24 merged,
do conda install --file requirements/docs.txt
cd docs && make html && open build/html/index.html
and commit the message
git add docs
git commit -m "skpkg: migrate documentation"

Copy link
Collaborator

@zmx27 zmx27 left a comment

Choose a reason for hiding this comment

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

See comments

AUTHORS.rst Outdated

Xiaohao Yang

Rundong Hua, Simon Billinge, Billinge Group members
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think just Billinge Group members (keep Xiaohao though)

LICENSE.rst Outdated
@@ -0,0 +1,41 @@
BSD 3-Clause License
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should just keep the contents of the old LICENSENOTICE.txt file, update it to .rst, and change the copyright years so it ends at our current year

README.rst Outdated

|PyPI| |Forge| |PythonVersion| |PR|

|CI| |Codecov| |Black| |Tracking|
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need the PyPI, Forge, CI, Codecov badges. They don't apply to this repo

Citation
--------

If you use diffpy.srxplanargui in a scientific publication, we would like you to cite this package as
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this should refer to the xpdfsuite paper written by Professor Simon

diffpy.srxplanargui Package, https://github.com/diffpy/diffpy.srxplanargui

Installation
------------
Copy link
Collaborator

Choose a reason for hiding this comment

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

Conda forge will not be how we install this package. This package will likely be installed with xpdfsuite or perhaps as a wheel file if we decide to do it that way. And of course, users can always install from sources

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, since it would not be released on conda and pypi, we probably don't need the pypi installation instruction either?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes

README.rst Outdated
---------------

You may consult our `online documentation <https://diffpy.github.io/diffpy.srxplanargui>`_ for tutorials and API references.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we will have an online documentation for this package

You may consult our `online documentation <https://diffpy.github.io/diffpy.srxplanargui>`_ for tutorials and API references.

Support and Contribute
----------------------
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this package will be open source

@@ -0,0 +1,56 @@
#######
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think that we will be making docs for this, but please check with Professor Simon.


**Fixed:**

* Support ``scikit-package`` Level 5 standard (https://scikit-package.github.io/scikit-package/).
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think also add how you did py2 to py3 conversion in this news file

@stevenhua0320
Copy link
Contributor Author

@zmx27 Ready for review for edited files (no doc index.rst edit yet, please review for the other files)

Copy link
Collaborator

@zmx27 zmx27 left a comment

Choose a reason for hiding this comment

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

See comments. I don't think we will be needing the docs folder for this package as it's likely intended to be very lightweight, but do check with Professor Simon like I've said

OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

Use of this software is subject to and permitted only under a separate,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we just need this part, along with the copyright years being 2009-2025. I believe that the BSD license is used for open source projects, which doesn't apply here. We usually don't want to change the license of our packages. Since LICENSENOTICE.txt already exists, maybe just modify that file and change the copyright dates

README.rst Outdated
.. |Codecov| image:: https://codecov.io/gh/diffpy/diffpy.srxplanargui/branch/main/graph/badge.svg
:target: https://codecov.io/gh/diffpy/diffpy.srxplanargui

.. |Forge| image:: https://img.shields.io/conda/vn/conda-forge/diffpy.srxplanargui
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can probably delete the unused icon sections like |Forge|, etc. Also delete the CI icon, because it won't render properly on private repos

README.rst Outdated
.. |PyPI| image:: https://img.shields.io/pypi/v/diffpy.srxplanargui
:target: https://pypi.org/project/diffpy.srxplanargui/

.. |PythonVersion| image:: https://img.shields.io/pypi/pyversions/diffpy.srxplanargui
Copy link
Collaborator

Choose a reason for hiding this comment

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

This won't render properly since this package isn't on PyPI. I usually like to replace this with
|PythonVersion| image:: https://img.shields.io/badge/python-3.11%20|%203.12%20|%203.13-blue
to artificially generate the supported python versions (3.11-3.13)

README.rst Outdated

REQUIREMENTS
------------------------------------------------------------------------
For more information about the diffpy.srxplanargui library, please consult our `online documentation <https://diffpy.github.io/diffpy.srxplanargui>`_.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we will have an online documentation for this package

diffpy.srxplanargui Package, https://github.com/diffpy/diffpy.srxplanargui

Installation
------------
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes

README.rst Outdated
Installation
------------

The preferred method is to be installed with ""xpdfsuite"" package or the wheel file.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor, but use `` to have it render as a "code" block

@stevenhua0320
Copy link
Contributor Author

@sbillinge Zhiming and I want to confirm that do we need documentation for this private package?

@stevenhua0320
Copy link
Contributor Author

@zmx27 Could you help me review if anywhere still needs edit for your comment files?

@sbillinge sbillinge merged commit 2fd5097 into diffpy:migration Oct 7, 2025
1 check failed
@sbillinge sbillinge deleted the doc branch October 7, 2025 21:53
@sbillinge
Copy link
Contributor

@stevenhua0320 I am merging this despite it failing tests as it contains no code. However, please can you make a PR with the objective of getting the tests to pass?

@stevenhua0320
Copy link
Contributor Author

@sbillinge I think we need to wait for the current open PR to be merged then we could have passing test. Since the .codespell and requirement folder are in current open PRs (different PRs). We need to merge those to have passing test.

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.

3 participants