-
Notifications
You must be signed in to change notification settings - Fork 177
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
Edits to improve functionality and documentation #889
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -200,9 +200,11 @@ def check_fisp_version(data): | |
|
||
|
||
def isFisII(data): | ||
"""boolean check if file is fispact-ii output """ | ||
v = check_fisp_version(data) | ||
if v == "FISPACT-II": | ||
"""boolean check if file is fispact-ii output (or fispact output for the program user's convinience)""" | ||
v = check_fisp_version(data) | ||
if v == "FISPACT": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A test is needed to check this case. Can you add a test that demonstrates the FISPACT (rather than FISPACT-II) behavior here? |
||
print("File is FISPACT instead of FISPACT-II") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than print, an exception or a warning is more appropriate here. Either raise an exception or execute a warning message with "warn" and return something (true or false) in this if v=FISPACT block. |
||
elif v == "FISPACT-II": | ||
return True | ||
else: | ||
return False | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -224,3 +224,5 @@ it is as easy as forking the repository on GitHub, making your changes, and | |
issuing a pull request. If you have any questions about this process don't | ||
hesitate to ask the mailing list (https://groups.google.com/forum/#!forum/pyne-dev, | ||
pyne-dev@googlegroups.com). | ||
|
||
Make sure to be appropriate on the forum and provide as clear of an explanation (with examples if possible) of any problems or questions you might have. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably add a code of conduct for pyne, but until then, "appropriate" is vague. So, perhaps the appropriate wording here is just "Please provide as clear of an explanation (with examples if possible) of any problems or questions you might have." |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,3 +23,4 @@ and IPython (v1.0+). Note that only `yt-3.0 <https://bitbucket.org/yt_analysis/y | |
supports PyNE. Be sure to use this version of yt rather | ||
than the more stable v2.x series. | ||
|
||
"""I suggest adding a reference for people to contact incase they have questions here: _________""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than have then contact people individually, I'd actually suggest that they submit a ticket in the issues system or send an email to pyne-dev. Perhaps the text you have above would be appropriate here: If you have any questions about this process, don't hesitate to ask the mailing list (https://groups.google.com/forum/#!forum/pyne-dev, pyne-dev@googlegroups.com). pyne-dev@googlegroups.com). Please provide as clear of an explanation (with examples if possible) of any problems or questions you might have. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
convinience → convenience. As you change this comment, you can fix "fispact-ii" into "FISPACT-II".