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

Move non-plotting modules to separate files in src directory #832

Merged
merged 9 commits into from
Feb 6, 2021

Conversation

seisman
Copy link
Member

@seisman seisman commented Feb 4, 2021

Description of proposed changes

Part 1 of fixing #807

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If adding new functionality, add an example to docstrings or tutorials.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

@seisman
Copy link
Member Author

seisman commented Feb 4, 2021

All tests still pass and the API documentation looks good.

@willschlitzer
Copy link
Contributor

Do we want to make separate folders and init.py files in the src folder to import the functions for mathops/gridops/etc.? There aren't a ton of non-plotting functions that are wrapped, but I figure this would keep the overall number of imports into the main pyGMT script low, especially as our numbers of wrapped modules increase.

@seisman
Copy link
Member Author

seisman commented Feb 4, 2021

Do we want to make separate folders and init.py files in the src folder to import the functions for mathops/gridops/etc.?

I don't think so. Like what you said in #807, if we make separate folders, we still have to decide "if grd2cpt.py should be under mathops/ or grdops/?"

@willschlitzer
Copy link
Contributor

Good point!

@seisman seisman added the maintenance Boring but important stuff for the core devs label Feb 4, 2021
@seisman seisman added this to the 0.3.0 milestone Feb 4, 2021
@seisman
Copy link
Member Author

seisman commented Feb 5, 2021

I almost finished the moving of non-plotting modules, except a few functions that need more discussion:

  • grdinfo: will move after Wrap grdinfo aliases #799 to avoid conflicts
  • config: wrapper of GMT's set command. pygmt/config.py? pygmt/src/config.py? pygmt/src/set.py?
  • GMTDataArrayAccessor: pygmt/datatypes.py?
  • x2sys_init and x2sys_cross: do we want to put these functions into two files or one file pygmt/src/x2sys.py?

@seisman
Copy link
Member Author

seisman commented Feb 5, 2021

Ping @GenericMappingTools/python and @GenericMappingTools/python-maintainers for comments on #832 (comment)

@weiji14
Copy link
Member

weiji14 commented Feb 6, 2021

I almost finished the moving of non-plotting modules, except a few functions that need more discussion:

Maybe split it into 2 PRs. Name this Part 1 and merge it first, and then we do the ones below in a Part 2 PR.

Agree. Or should we just finish this PR (#832) and move grdinfo directly under src/ in #799?

  • config: wrapper of GMT's set command. pygmt/config.py? pygmt/src/config.py? pygmt/src/set.py?
  • GMTDataArrayAccessor: pygmt/datatypes.py?

I would leave these in modules.py for now since they're both classes. We can review them for v0.4.0.

  • x2sys_init and x2sys_cross: do we want to put these functions into two files or one file pygmt/src/x2sys.py?

This is a hard one. There are a few other x2sys modules which are still unwrapped. I would keep them in one file for now (keeps the diff small), but probably need to split them up in the future.

@seisman seisman changed the title WIP: Move non-plotting modules to separate files Move non-plotting modules to separate files Feb 6, 2021
@seisman seisman changed the title Move non-plotting modules to separate files Move non-plotting modules to separate files in src directory Feb 6, 2021
@seisman seisman marked this pull request as ready for review February 6, 2021 00:32
@seisman
Copy link
Member Author

seisman commented Feb 6, 2021

Maybe split it into 2 PRs. Name this Part 1 and merge it first, and then we do the ones below in a Part 2 PR.

Sounds good. Now this PR is ready for review.

Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

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

Cool, thanks @seisman, not easy moving so many files! I see a lot of places where we could use f-strings and update the documentation following the convention agreed in #775 but those can be addressed in the future to keep the diff small here.

@seisman seisman merged commit f62218d into master Feb 6, 2021
@seisman seisman deleted the moving-modules branch February 6, 2021 01:01
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
…MappingTools#832)

* Move blockmedian to src
* Move grdtrack to src
* Move surface to src
* Move makecpt to src
* Move grdfilter to src
* Move grdcut to src
* Move info to src
* Move which to src
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants