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

File list update #1

Merged
merged 6 commits into from
Jun 20, 2019
Merged

Conversation

rogiersbart
Copy link

@rogiersbart rogiersbart commented Jun 19, 2019

Cas, please consider pulling in changes from rogiersbart/RMODFLOW, develop branch asap. Should be straightforward if everything you did is on your remote feature/input-structure branch. I have a little bit of time now for the package, and I'd like to streamline the development workflow somewhat. Also hope to create a bunch of issues on github soon to discuss the way forward.

@rogiersbart
Copy link
Author

Ok, I see you just updated your feature branch. This pull request doesn't make much sense anymore now I guess. Is more coming? Or is this everything you had locally? If so, I might try to integrate it with the new files, so we can start from a clean slate again ... No problem if you want to do it of course.

@cneyens cneyens merged commit 2019c39 into cneyens:feature/input-structure Jun 20, 2019
@cneyens
Copy link
Owner

cneyens commented Jun 20, 2019

Hi Bart,

Weirdly enough, I did not get a notification for this pull request so I'm only seeing this now. I've reverted my commits on the feature/input-structure branch back to the point when you made this pull request and merged so my feature branch is now up-to-date with your develop branch.

The commits that I made and subsequently reverted are indeed part of a larger pull-request that I was working on that's nearly finished. The main scope was to have a smoother way of creating RMODFLOW boundary-condition and flow packages e.g.:

rmf_create_array(0.04, dim = c(dis$nrow, dis$ncol), kper = c(1,3)) %>%
 rmf_create_rch(dis = dis)

The rmf_create_* functions would then take care of the rest, i.e. setting the proper dimensions etc.
instead of the very verbose input some of the rmf_create_* functions need as they are now. This new input format would also open the door to an easier implementation with sf and stars for spatial data, something like:

sf::st_read("river.shp") %>%
 rmf_create_list(kper = 1:dis$nper, dis = dis) %>%
 rmf_create_riv(dis = dis)

The reason this has been delayed is due to 1) the time it takes to come up with the easiest and most user-friendly way to implement these type of functions and 2) the difficulties that come along with MODFLOW parameters.

So indeed there is more to come on this front. Basically, all base MODFLOW-2005 & NWT packages are almost ready minus S3 methods for plotting. I've also worked on some vignettes to explain these functions and the design choices (one of them is already pushed). It's up to you to see what you want to take or edit from this of course.

If it's ok with you, I can add these commits back to this feature branch (keeping in mind the new file list format) and create a pull-request with your develop branch once I'm done.

In terms of opening issues, I would strongly suggest creating tests and setting up Travis CI seeing as the package is getting bigger and bigger...

@rogiersbart
Copy link
Author

Ok Cas. Thanks for making the effort to merge everything. Looking forward to see the rest of this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants