duncan-r / ship Goto Github PK

These were in for convenience in an early internal version of the library. They don't make a lot of sense being there any more.

Perhaps mark as deprecated?

@JamesRamm I thought I'd start a separate issue for this so we can keep on top of the progress separately from the general updates.

There is quite good coverage of this in the existing library, although some are not well catered for. There can be a lot of different types of "DEFINE" block that represent different things. For instance:
DEFINE Event
DEFINE OutputZone

To complicate this some stuff like Event can be added using DEFINE, as above, or using if-else logic:
IF EVENT == .

There are also domain specific clauses (these are not currently supported):
START 1D Domain
START 2D Domain

A lot of this can be a little problematic, because certain combinations of words will change the meaning. They will probably need separate rules (regex's) under the new system and probably an easy way to add in additional ones as they tend to get added fairly often (I think DEFINE Output Zone was only added a year or so ago).

The slides from this talk by Bill covers a lot of it quite well, but it's not complete and there have been some updates since this was made.
https://www.tuflow.com/Download/Presentations/2012/2012%20Aust%20Workshops%20-%20TUFLOW%20Multiple%20Events%20and%20Scenarios.pdf

It may be worth while contacting TUFLOW to try and get a complete list of these?

I've been considering this for a while and it's been highlighted a little by the updates in #16 .

The fmp classes hold a PathHolder member, rather than inheriting from PathHolder directly. This may be preferable for the TuflowFile classes as well.

I've considered before whether the different types of ATuflowPart could in fact be simplified a lot and given a keyword type or some such. The command and variable/associates/filepath/etc could then be data/class members.

This could be particularly useful when updating commands that are similar in all ways except an argument on the left of the assignment (e.g. Read GIS Zline THICK == ..\somefile.mid where THICK or THIN are example arguments). It may also help with piped commands, which could potentially be a single part with multiple PathHolder members, although I will have to have a think about this.

It would probably remove some of the complication introduced by the MRO in places and be a bit more "pythonic".

Hi
Would you be amenable to a few changes aimed toward simplifying parsing the tuflow control files (specifically tuflowfactory.py)? My aim is to improve readability and ease of maintenance.
The initial changes I'd like to make are:

• replace breakLine and separateComment with a single regex: (.+?)(?:==)(.+)(?:[!|#].*?). This will capture the command and variable, while ignoring any trailing comments and the assignment operator.
  E.g:

pat = r'(.+?)(?:==)(.+)(?:[!|#].*?)'
line = ''Set IWL == 36.5	! Define an initial 2D water level of 36.5m"
result = re.findall(pat, line)
# result is [("Set IWL ", " 36.5 " )]

(I think some commands can have some extra info specified between brackets, which would need to be ignored...im not certain on this)

• Make TuflowFactory class a module/individual functions. There are a couple of problems with having it a class:

  • The classmethod doesnt actually return an instance of the class
  • The remaining methods are all static methods - there is no self use anywhere in the class
    Having it as separate functions would enhance readability. getTuflowPart would be the public api, remaining functions would be private.

• Check filepart types after parsing the line and splitting it into command/variable parts. This will reduce repetition of code and make it slightly simpler to match the file part types (could just do a dictionary or set lookup against the parsed command)

These modules were never fully updated after the restructure of the rest of the tuflow package.

The datafileloader.py classes often fail while loading bcdbase/tmf/etc file contents for a range of reasons, usually to do with misunderstandings in the flexibility that TUFLOW allows for these file configurations. This is particularly true for the subfile loaders (i.e. readMatSubfile()).

Some thoughts on the design of the loaders and potentially the ADataFileObject type classes are needed. The loaders in particular are overly complicated, hard to test and hard to maintain.

The biggest problem here is getting the data in to a format to able to write the files back out again, rather than read them in. TUFLOW allows users to either order columns with any header text in a specific sequence, or to provide specific headers in any order. Discovering this and maintaining the text of any original header is complicated. Perhaps it would be better to just work out what the columns contain and always use default header text when writing them out?

In addition to this __event__ style variables need to be considered as well.

See TUFLOW Manual.2016-03-AA.pdf - section 7.5 on page 7-28 for more information on BC Database file configuration.

Continuing from #2

I think that some changes to the API for the tuflow package are almost inevitable here. The public side is not particularly well defined at the moment.

I think we need to probably get this completely nailed down, tag the dev branch pre-merge, but with the cross platform fixes in and then make the break.

Some thoughts on #2 (comment) .
If we replace the functionality in some places like ControlFile with ControlStructure I think it would be best to keep the existing naming convention. They match the tuflow naming conventions (TuflowModel, ControlFile, etc) and are therefore familiar to users.

I think importantly I'd like to maintain the flexibility of the tuflow package, but with the added bonus of making it a bit nicer to maintain and a bit more understandable for users. This includes:

• Making sure we can handle parts of files that haven't been specifically handled in the library yet.
• Allow reading/writing/updating of the different files.
• Allow interrogating/updating/removing different parts of the files. It should also be possible to hand in specific scenario/event vals to change the way these are returned.
• Allow grouping of different components (gis files/control files/variables with certain commands etc)
• Hopefully the ability to add/remove/replace certain control files.
• Allow updating the logic structures (Moving files in or out of them, removing them (with or without the parts they contain)), and adding them in.
• Updating file paths, both the root location (.tcf folder) and relative links to certain files (.tgc/etc and gis files etc)

This is all stuff that is currently supported and definitely used. There are probably other useful inclusions too. To have it all, but make it a bit nicer to use would be great.

Following on from the discussion from #4:

It appears as though we need changes to filetools.py and the unit tests in order to properly support cross platform handling of the file paths.

Integration tests in Travis have been deactivated temporarily, until the changes to the fileloader and tuflowfactory are complete.

The integration tests will need updating and reactivating when everything calms down.

The majority of super class calls use the old style ClassName.method(self, *args, **kwargs) format.
These need updating to super(ClassName, self).method(*args, **kwargs)

Note that the super().method(*args, **kwargs) can't be used because we need to maintain v2.7 compatibility.

Overarching task for integration with anuga hydro.