Skip to content

docs: add the instruction to reinstall pycifrw #145

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

Closed
wants to merge 1 commit into from

Conversation

ycexiao
Copy link
Contributor

@ycexiao ycexiao commented Jul 23, 2025

What problem does this PR address

Adding the instruction to reinstall pycifrw as mentioned in #144

What should the reviewer(s) do

Please check the modified README.rst

@ycexiao ycexiao marked this pull request as ready for review July 23, 2025 21:51
Copy link

codecov bot commented Jul 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.93%. Comparing base (9379e80) to head (b087ea3).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #145   +/-   ##
=======================================
  Coverage   98.93%   98.93%           
=======================================
  Files          13       13           
  Lines        1878     1878           
=======================================
  Hits         1858     1858           
  Misses         20       20           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ycexiao
Copy link
Contributor Author

ycexiao commented Jul 23, 2025

@sbillinge, it's ready for review.

Copy link
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

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

this is good, but please see my comments

@@ -99,6 +99,10 @@ and run the following ::

pip install .

In ``diffpy.structure>=3.3.1``, please ensure the dependency ``pycifrw`` is installed from ``conda-froge`` ::
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do a bit more checking. Are the PyPI and conda version numbers the same? if not, then it could be a version number issue. and the instructions would be to install pycifrw>=??? or pycifrw<=??. If the version is the same on PyPI and conda-forge, let the users know that explicitly (the conda-forge one works but the PyPI one doesn't).

We can also check to see if we can update our tests so it passes with both versions too.

Let's discuss it, but we can also propose to make an issue/PR at pycifrw.

@sbillinge
Copy link
Contributor

closed as not needed. Closed by the fix in #146

@sbillinge
Copy link
Contributor

closed as not needed. Closed by the fix in #146

@sbillinge sbillinge closed this Jul 23, 2025
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.

2 participants