Skip to content

Hard coded MXG in MMS gridfile generator made flexible modified: mms.py#280

Merged
ZedThree merged 3 commits intoboutproject:masterfrom
JensMadsen:adding_mms_flex
Oct 6, 2016
Merged

Hard coded MXG in MMS gridfile generator made flexible modified: mms.py#280
ZedThree merged 3 commits intoboutproject:masterfrom
JensMadsen:adding_mms_flex

Conversation

@JensMadsen
Copy link
Contributor

When writing grid files for MMS use MXG=2 was hard coded. MXG is now an input parameters with default value 2.

When writing grid files for MMS use MXG=2 was hard coded. MXG is now an input parameters with default value 2.
Copy link
Contributor

@dschwoerer dschwoerer left a comment

Choose a reason for hiding this comment

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

In my opinion omitting those two lines makes the code more readable.

"""

MXG = 2
MXG = MXG
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this line can be removed?

"""

MXG = 2
MXG = MXG
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes of course. Sorry. I am not that familiar with the fork-branch-pull_request workflow.... Am I supposed to make make the changes and make a new pull request ?

Copy link
Member

Choose a reason for hiding this comment

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

You can make the changes and push to the same branch (JensMadsen/adding_mms_flex) and it will automatically be added to this pull request.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you make the changes and push them, the pull request will automatically be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I do that.
....
By the way. I was changing a minor thing in manual/tex_files/coordinates.tex as well, but in another branch. What is the best way to do this? two different pull request because the changes are not related or just in one pull request because the changes are tiny?



def write(self, nx, ny, output):
def write(self, nx, ny, MXG = 2, output):
Copy link
Member

Choose a reason for hiding this comment

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

Can you change the def to this (likewise on L510):

def write(self, nx, ny, output, MXG=2):

Arguments with defaults should go at the end, preferably. Also, there shouldn't be spaces around the equals in the argument definition.

@ZedThree
Copy link
Member

ZedThree commented Oct 4, 2016

Two separate pull requests are fine

On Tue, 4 Oct 2016, 14:45 Jens Madsen, notifications@github.com wrote:

@JensMadsen commented on this pull request.

In tools/pylib/boutdata/mms.py
#280:

@@ -321,7 +321,7 @@ def write(self, nx, ny, output):

     """
  •    MXG = 2
    
  •    MXG = MXG
    

Thanks. I do that.
....
By the way. I was changing a minor thing in
manual/tex_files/coordinates.tex as well, but in another branch. What is
the best way to do this? two different pull request because the changes are
not related or just in one pull request because the changes are tiny?


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#280, or mute the thread
https://github.com/notifications/unsubscribe-auth/ABawXp7D6nnZb42YSdXCrxhEKc4q5mh-ks5qwlhxgaJpZM4KNp_v
.

	modified:   mms.py

removed unnecessary MXG = MXG. MXG argument which has a default value now comes after arguments without default values.
Copy link
Member

@ZedThree ZedThree left a comment

Choose a reason for hiding this comment

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

Please could you also add output/filename and MXG to the docstrings? :)

Added descriptions of 'output' and 'MXG' parameters to the docstrings of the write member function of the classes 'SimpleTokamak'. 'ShapedTokamak'.
@JensMadsen
Copy link
Contributor Author

docstring updated

@ZedThree ZedThree merged commit ee45fef into boutproject:master Oct 6, 2016
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