Opened 7 years ago

Closed 7 years ago

#1889 closed defect (fixed)

wxPy loc'n wiz: don't auto-launch into PERMANENT

Reported by: hamish Owned by: grass-dev@…
Priority: blocker Milestone: 6.4.3
Component: wxGUI Version: 6.4.3 RCs
Keywords: location wizard Cc:
CPU: x86-64 Platform: Unspecified

Description

Hi,

a change a while back changed things so that after you complete setting up a new location, and after it asks you if you want to set the default bounds, it automatically launches in the PERMANENT mapset.

Day to day use of PERMANENT as your working mapset is not good practice and the auto-launch behaviour should be reverted. Instead it should act like the text location creation tool and ask you if you want to create a new mapset, auto-filling in the $user name as a suggestion for what to call it.

If "create mapset" name is left blank, it should quietly return to the location/mapset as if you'd pressed cancel, instead of giving a python error. From there you can work with just PERMANENT if you are really dead-set on doing that. If an invalid name is given it should give an error message about that and either return you to the create new mapset prompt or the main startup window.

Then back on the startup window it should present you with that mapset highlighted and leave you to press "start grass". e.g., what if you have a new system and want to set up 10 new locations? I do appreciate the spirit of helpfulness and assisting with the workflow, but it shouldn't go so far as executing decisions on the users' behalf.

Perhaps this came in with the code to automatically import the georeferenced map you created a new location from? (and that one should ask you first, e.g. the geofile may be massive and you don't want to import it)

thanks, Hamish

ps- the location wizard, on the whole, continues to be really awesome and a brilliant first impression for new users

Change History (23)

comment:1 Changed 7 years ago by marisn

Platform: MSWindows 7Unspecified

Hamish, You were faster today. I would also suggest to go back to previous behaviour when user was taken back to start page. It's possible to preselect new location and PERMANENT mapset to save clicks, but it is also giving ability to create more mapsets and work in one of them.

comment:2 in reply to:  description Changed 7 years ago by annakrat

When I create new location, I usually want to import data first. So launching in PERMANENT seems to be reasonable from this point of view.

Replying to hamish:

Perhaps this came in with the code to automatically import the georeferenced map you created a new location from? (and that one should ask you first, e.g. the geofile may be massive and you don't want to import it)

It asks.

comment:3 Changed 7 years ago by hamish

When I create new location, I usually want to import data first.

perhaps, but that's not a decision that should be made for someone else. Guiding people to use PERMANENT as their working mapset by default is not good, and that far outweighs any one-off convenience + is dysfunctional in a multi-user/team setting.

e.g. the geofile may be massive and you don't want to import it

It asks.

ok, thanks.

best, Hamish

comment:4 Changed 7 years ago by annakrat

Any other opinions on this?

Anna

comment:5 in reply to:  4 Changed 7 years ago by mlennert

Replying to annakrat:

Any other opinions on this?

I am 100% with Hamish: automatically entering a mapset without having the user explicitely ask for it is a very bad idea IMHO. Is one click really too much to ask of a user ?

Moritz

comment:6 Changed 7 years ago by helena

I agree with Hamish and Moritz on this, Helena

comment:7 Changed 7 years ago by annakrat

Please try r55185. It asks for new mapset after creating location and goes back to startup window.

I noticed that startup window gui accepts non-ascii mapset and location name but later, the gui complains about it. See #1293. I suggest to check the input and do not allow diacritic.

Regards, Anna

comment:8 in reply to:  7 ; Changed 7 years ago by mlennert

Replying to annakrat:

Please try r55185. It asks for new mapset after creating location and goes back to startup window.

This looks good to me, now. We might want to think about a tooltip that explains why creating a mapset (other than PERMANENT) is a good idea, but that's cosmetics.

I noticed that startup window gui accepts non-ascii mapset and location name but later, the gui complains about it. See #1293. I suggest to check the input and do not allow diacritic.

+1

Is there a possibility to call a Python equivalent of G_legal_name() ?

Moritz

comment:9 in reply to:  8 Changed 7 years ago by glynn

Replying to mlennert:

Is there a possibility to call a Python equivalent of G_legal_name() ?

I don't know whether there's anything already in wxGUI, but I've added a legal_name() function to grass.script.core.

comment:10 Changed 7 years ago by wenzeslaus

In r55231 I added some doc for r55229 but actually, I don't understand the issue completely. I know about ticked #1293 and I can see that functions G_legal_filename or G_check_input_output_name (file legal_name.c) are used only on a few places (according to grep and programming manual). There is some check in G_parser function because it is not possible to create map with dot or slash but probably G_legal_filename is not used. (There is a warning which is reported twice but that is another issue.)

I've tried to create map with non-latin characters but I'm not able to do it. wxGUI fails with some bugs and command line (Bash Shell) does not accept my characters and it does some other things instead such as cleaning the line.

comment:11 in reply to:  10 Changed 7 years ago by glynn

Replying to wenzeslaus:

In r55231 I added some doc for r55229 but actually, I don't understand the issue completely. I know about ticked #1293 and I can see that functions G_legal_filename or G_check_input_output_name (file legal_name.c) are used only on a few places (according to grep and programming manual). There is some check in G_parser function because it is not possible to create map with dot or slash but probably G_legal_filename is not used. (There is a warning which is reported twice but that is another issue.)

G_parser() doesn't check for legal file names. However, G_open() etc do, and those functions are used by Rast_open_new() etc.

I've tried to create map with non-latin characters but I'm not able to do it.

Bytes above 126 aren't allowed in filenames. That excludes the DEL character as well as all non-ASCII characters, regardless of the encoding.

comment:12 in reply to:  8 Changed 7 years ago by annakrat

Replying to mlennert:

This looks good to me, now. We might want to think about a tooltip that explains why creating a mapset (other than PERMANENT) is a good idea, but that's cosmetics.

I would put it directly in the dialog message here.

I noticed that startup window gui accepts non-ascii mapset and location name but later, the gui complains about it. See #1293. I suggest to check the input and do not allow diacritic.

+1

Done in r55278 using legal_name function. It applies for creating new location/mapset and changing their names. Please test (this is a blocker).

comment:13 in reply to:  description ; Changed 7 years ago by hellik

tested with

GRASS version: 6.4.3svn                                                         
GRASS SVN Revision: 55262                                                       
GIS Library Revision: 50937 (2012-02-25)                                        
GDAL/OGR: 1.9.2                                                                 
PROJ4: Rel. 4.8.0, 6 March 2012                                                 
Python: 2.7.2                                                                   
wxPython: 2.8.12.1                                                              
Platform: Windows-7-6.1.7601-SP1 (OSGeo4W)

Replying to hamish:

Hi,

a change a while back changed things so that after you complete setting up a new location, and after it asks you if you want to set the default bounds, it automatically launches in the PERMANENT mapset.

it doesn't anymore now.

Day to day use of PERMANENT as your working mapset is not good practice and the auto-launch behaviour should be reverted. Instead it should act like the text location creation tool and ask you if you want to create a new mapset, auto-filling in the $user name as a suggestion for what to call it.

it does now.

Perhaps this came in with the code to automatically import the georeferenced map you created a new location from? (and that one should ask you first, e.g. the geofile may be massive and you don't want to import it)

it asks for importing the georeferenced map.

closing the ticket?

Helmut

comment:14 Changed 7 years ago by hamish

Hi,

just tested with a fresh build of relbr64. After setting the region bounds it is still printing some g.region debug output to the terminal.

when the create new mapset gui pops up, it would be good to have a few sentence tooltip available explaining what MAPSET means, and a [Help] button at the bottom that opened up $GISBASE/docs/html/helptext.html.

closing the ticket?

not before it has been ported to all branches. (please don't do development work in the release branch and then not sync with the dev branches. The release branch must be treated with great care, only code tested in the relevant dev branch should be backported to the release branch- by backporting it to the release branch you're personally signing off that the code is tested and works. tx)

thanks, Hamish

comment:15 in reply to:  14 Changed 7 years ago by annakrat

Replying to hamish:

Hi,

just tested with a fresh build of relbr64. After setting the region bounds it is still printing some g.region debug output to the terminal.

There is flag p in the g.region command (here). Intention?

when the create new mapset gui pops up, it would be good to have a few sentence tooltip available explaining what MAPSET means, and a [Help] button at the bottom that opened up $GISBASE/docs/html/helptext.html.

Help button added in r55297. Please could you add the help text (I wouldn't use tooltip, just add another sentence to the message)?

closing the ticket?

not before it has been ported to all branches. (please don't do development work in the release branch and then not sync with the dev branches. The release branch must be treated with great care, only code tested in the relevant dev branch should be backported to the release branch- by backporting it to the release branch you're personally signing off that the code is tested and works. tx)

Sure, I just wanted to wait for all the changes.

Anna

comment:16 in reply to:  14 Changed 7 years ago by martinl

Replying to hamish:

not before it has been ported to all branches. (please don't do development work in the release branch and then not sync with the dev branches. The release branch must be treated with great care, only code tested in the relevant dev branch should be backported to the release branch- by backporting it to the release branch you're personally signing off that the code is tested and works. tx)

just small note: it's probably related to the fact, that you are probably only developer who wants to do "development" in GRASS 6 branches. There were several discussions in the past, and I don't want to open this topic again. The result will the same. Please bear in mind, that you are probably only one from devs who really wants to keep develbranch_6 alive. Development should be done only in trunk! Anyway this is one of consequences, all of us need to maintain three branches just because we didn't decided that we don't need develbranch_6. This is meant to open discussion again, it's just a reminder.

comment:17 in reply to:  14 ; Changed 7 years ago by annakrat

Replying to hamish:

not before it has been ported to all branches. (please don't do development work in the release branch and then not sync with the dev branches. The release branch must be treated with great care, only code tested in the relevant dev branch should be backported to the release branch- by backporting it to the release branch you're personally signing off that the code is tested and works. tx)

Also, I did the changes first in release branch because now it's not time to test it elsewhere, then backport and test it again.

Btw, this 'blocker' could have been opened a long time ago and not just before release.

comment:18 in reply to:  17 ; Changed 7 years ago by martinl

Replying to annakrat:

Also, I did the changes first in release branch because now it's not time to test it elsewhere, then backport and test it again.

Btw, this 'blocker' could have been opened a long time ago and not just before release.

good point, more over there were open four blockers in one day by one person just when we claimed to release RC3 and then final. We should really change policy in this sense. Ideally should come two RC in shorter period (two or three weeks) and then final. Currently we have RC1 in October, RC2 in December. RC3 who knows. Sometimes I have impression that there are people who don't care about releases (which is acceptable) and people who are in the result blocking (or making releasing process painful). One release if usually story for half of the year or more (see number of RC and time). It would be nice to have more releases (two per year) with smaller resistance. Just my 2 cents. Not related to this ticket. My personal opinion which shouldn't be reported here.

comment:19 in reply to:  18 Changed 7 years ago by wenzeslaus

There is nothing wrong with reporting issues. The point is that we must decide what is the blocker and what is only a feature request. Moreover, some issues, blockers or non-blockers, are just only for some other release, not for the current. The decision which type, priority and release is associated with each ticket can be made by anyone, not only the original reporter.

comment:20 Changed 7 years ago by marisn

This is a wrong place where to discuss GRASS project issues.

Still, as it's too late, I'll drop in my rant: The problem is that we have too big changes in 6.4.x. Most of blockers are in features that were not existing in 6.4.0, 6.4.1. As fixing bugs sometimes involves introducing new code and new features, finding new bugs is unstoppable. Want an example? Take look at "identify" problems of vectors. In 6.4.0 it was plain v.what with printing in console. Pretty? No, but worked well. Now try to count how many blockers/critical bugs were filled against the new identify tool. As long as we will continue to introduce new features/change existing behaviour (i.e. import and launch into PERMANENT) just 5 min. before release, we will have blockers just 1 minute before release. I'm sorry, I don't have any free time for GRASS testing. I'm (and probably others too) reporting issues when I find them during ordinary work or when my labs fail because of "new and cool feature".

comment:21 in reply to:  20 Changed 7 years ago by cmbarton

Replying to marisn:

This is a wrong place where to discuss GRASS project issues.

Still, as it's too late, I'll drop in my rant: The problem is that we have too big changes in 6.4.x. Most of blockers are in features that were not existing in 6.4.0, 6.4.1. As fixing bugs sometimes involves introducing new code and new features, finding new bugs is unstoppable. Want an example? Take look at "identify" problems of vectors. In 6.4.0 it was plain v.what with printing in console. Pretty? No, but worked well. Now try to count how many blockers/critical bugs were filled against the new identify tool. As long as we will continue to introduce new features/change existing behaviour (i.e. import and launch into PERMANENT) just 5 min. before release, we will have blockers just 1 minute before release. I'm sorry, I don't have any free time for GRASS testing. I'm (and probably others too) reporting issues when I find them during ordinary work or when my labs fail because of "new and cool feature".

I also am VERY supportive of getting releases out more rapidly. But I know where Maris is coming from. I try to test any any new features as soon as I can on the Mac. But the frustrating thing is when I discover (in the process of doing work or teaching) things that USED to work, now don't due to some upstream change. Especially when working with release candidates, it is essential to test any changes thoroughly when committing or backporting. It's hard enough to test and fix existing bugs and new features, but having previously working code broken makes vetting a really tough task. More testers always help too.

comment:22 in reply to:  13 ; Changed 7 years ago by hellik

Replying to hellik:

closing the ticket?

?

Helmut

comment:23 in reply to:  22 Changed 7 years ago by annakrat

Resolution: fixed
Status: newclosed

Replying to hellik:

closing the ticket?

All changes applied also in 6.5 (r55314) and 7 (r55312). Closing the ticket.

Note: See TracTickets for help on using tickets.