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

Add clarification #96

Closed
wants to merge 1 commit into from
Closed

Add clarification #96

wants to merge 1 commit into from

Conversation

Fil158
Copy link

@Fil158 Fil158 commented Oct 8, 2024

I think is better to add some more comment to clarify the description

I think is better to add some more comment to clarify the description
Copy link

github-actions bot commented Oct 8, 2024

Warning! No news item is found for this PR. If this is a user-facing change/feature/fix,
please add a news item by copying the format from news/TEMPLATE.rst.

@bobleesj
Copy link
Contributor

bobleesj commented Oct 8, 2024

@Filippo1598 Thanks. For the issue title, it would be good to add where at the title level so that we can quickly look through the issues at once. Ex) Clarify long description in README.rst

Also, please close this PR since we want an issue instead.

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.

Thanks for this @Filippo1598 . Please see my online comment

@@ -45,6 +45,8 @@ on the independent variable axis. For example, for powder diffraction data taken
chemical phases where the measurements were done at different temperatures and the materials were undergoing thermal
expansion.

I think it is not very clear here what the package does and how it can be used to solve the example problem. You are talking about VT-PXRD experiment of samples with different phases which undergo a termal expansion, so a modification of unit cell parameters, each phase independently with respect to the other in the same sample?
Copy link
Contributor

Choose a reason for hiding this comment

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

@Filippo1598 this is a comment, not a fix. Please put comments in the comments section on GitHub and only fixes in the code itself. What would be needed here is your proposed text. I'm this case we may want to discuss it a bit so drafting text in the comments section and then editing it over there before pasting it here may be a preferred workflow.

@Fil158 Fil158 closed this Oct 8, 2024
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